Message ID | 1409710691-22816-1-git-send-email-mike.holmes@linaro.org |
---|---|
State | Rejected |
Headers | show |
On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > > This creates a new section in the documentaion to group everything > related to buffers. It appears to make things much more acessable > although it needs to have some real description added to the section. > Looks better to me. It also allows more freedom within the implementation to place things in different header files and still have the documentation look the same. Small nit, I think it would be better to use defgroup and ingroup, rather than addtogroup, to avoid any confusion about where the documentation for that group should go (what happens if you document multiple addtogroups?).
On 4 September 2014 03:10, Stuart Haslam <stuart.haslam@arm.com> wrote: > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> --- >> >> This creates a new section in the documentaion to group everything >> related to buffers. It appears to make things much more acessable >> although it needs to have some real description added to the section. >> > > Looks better to me. It also allows more freedom within the implementation > to place things in different header files and still have the documentation > look the same. If implementations will have freedom to place things in different headers, how one app source will work with all of them? Which header it would include? I am not against fancy doxygen syntax, but mapping ODP symbol to header file name should be normative part of ODP. Of course, such mapping could be supported with set of implementation defined headers that are internally included by one that constitute ODP API. Thanks, Victor > Small nit, I think it would be better to use defgroup and ingroup, rather > than addtogroup, to avoid any confusion about where the documentation for > that group should go (what happens if you document multiple addtogroups?). > > -- > Stuart. > >> platform/linux-generic/include/api/odp_buffer.h | 6 +++++- >> platform/linux-generic/include/api/odp_buffer_pool.h | 6 ++++++ >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/platform/linux-generic/include/api/odp_buffer.h b/platform/linux-generic/include/api/odp_buffer.h >> index d8577fd..4f348e4 100644 >> --- a/platform/linux-generic/include/api/odp_buffer.h >> +++ b/platform/linux-generic/include/api/odp_buffer.h >> @@ -21,7 +21,10 @@ extern "C" { >> >> #include <odp_std_types.h> >> >> - >> +/** >> + * @addtogroup buffer >> + * @{ >> + */ >> >> /** >> * ODP buffer >> @@ -83,6 +86,7 @@ int odp_buffer_is_valid(odp_buffer_t buf); >> */ >> void odp_buffer_print(odp_buffer_t buf); >> >> +/** @} */ >> >> #ifdef __cplusplus >> } >> diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h b/platform/linux-generic/include/api/odp_buffer_pool.h >> index fe88898..dd0ac43 100644 >> --- a/platform/linux-generic/include/api/odp_buffer_pool.h >> +++ b/platform/linux-generic/include/api/odp_buffer_pool.h >> @@ -23,6 +23,11 @@ extern "C" { >> #include <odp_std_types.h> >> #include <odp_buffer.h> >> >> +/** >> + * @addtogroup buffer ODP Buffers and buffer Pools >> + * @{ >> + */ >> + >> /** Maximum queue name lenght in chars */ >> #define ODP_BUFFER_POOL_NAME_LEN 32 >> >> @@ -99,6 +104,7 @@ void odp_buffer_free(odp_buffer_t buf); >> */ >> odp_buffer_pool_t odp_buffer_pool(odp_buffer_t buf); >> >> +/** @} */ >> >> #ifdef __cplusplus >> } >> -- >> 1.9.1 >> > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On Thu, Sep 04, 2014 at 03:41:17PM +0100, Victor Kamensky wrote: > On 4 September 2014 03:10, Stuart Haslam <stuart.haslam@arm.com> wrote: > > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: > >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > >> --- > >> > >> This creates a new section in the documentaion to group everything > >> related to buffers. It appears to make things much more acessable > >> although it needs to have some real description added to the section. > >> > > > > Looks better to me. It also allows more freedom within the implementation > > to place things in different header files and still have the documentation > > look the same. > > If implementations will have freedom to place things in different headers, > how one app source will work with all of them? Which header it would > include? I am not against fancy doxygen syntax, but mapping ODP > symbol to header file name should be normative part of ODP. Of course, > such mapping could be supported with set of implementation defined > headers that are internally included by one that constitute ODP API. > > Thanks, > Victor > This has come up before and I think the intention has always been that the application should just include odp.h and the implementation can do whatever it likes with header files beyond that (except perhaps that they need to be named odp_*.h). Personally though, I would rather the file names were fixed, and I did wonder when writing that comment whether this decision should be revisited.
On 4 September 2014 06:10, Stuart Haslam <stuart.haslam@arm.com> wrote: > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > > > This creates a new section in the documentaion to group everything > > related to buffers. It appears to make things much more acessable > > although it needs to have some real description added to the section. > > > > Looks better to me. It also allows more freedom within the implementation > to place things in different header files and still have the documentation > look the same. > > Small nit, I think it would be better to use defgroup and ingroup, rather > than addtogroup, to avoid any confusion about where the documentation for > that group should go (what happens if you document multiple addtogroups?). > We can make it explicit, I was thinking that it was easier to manage with the looser association. multiple add groups just concatenate without a warning. > -- > Stuart. > > > platform/linux-generic/include/api/odp_buffer.h | 6 +++++- > > platform/linux-generic/include/api/odp_buffer_pool.h | 6 ++++++ > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_buffer.h > b/platform/linux-generic/include/api/odp_buffer.h > > index d8577fd..4f348e4 100644 > > --- a/platform/linux-generic/include/api/odp_buffer.h > > +++ b/platform/linux-generic/include/api/odp_buffer.h > > @@ -21,7 +21,10 @@ extern "C" { > > > > #include <odp_std_types.h> > > > > - > > +/** > > + * @addtogroup buffer > > + * @{ > > + */ > > > > /** > > * ODP buffer > > @@ -83,6 +86,7 @@ int odp_buffer_is_valid(odp_buffer_t buf); > > */ > > void odp_buffer_print(odp_buffer_t buf); > > > > +/** @} */ > > > > #ifdef __cplusplus > > } > > diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h > b/platform/linux-generic/include/api/odp_buffer_pool.h > > index fe88898..dd0ac43 100644 > > --- a/platform/linux-generic/include/api/odp_buffer_pool.h > > +++ b/platform/linux-generic/include/api/odp_buffer_pool.h > > @@ -23,6 +23,11 @@ extern "C" { > > #include <odp_std_types.h> > > #include <odp_buffer.h> > > > > +/** > > + * @addtogroup buffer ODP Buffers and buffer Pools > > + * @{ > > + */ > > + > > /** Maximum queue name lenght in chars */ > > #define ODP_BUFFER_POOL_NAME_LEN 32 > > > > @@ -99,6 +104,7 @@ void odp_buffer_free(odp_buffer_t buf); > > */ > > odp_buffer_pool_t odp_buffer_pool(odp_buffer_t buf); > > > > +/** @} */ > > > > #ifdef __cplusplus > > } > > -- > > 1.9.1 > > > > >
On 4 September 2014 11:01, Stuart Haslam <stuart.haslam@arm.com> wrote: > On Thu, Sep 04, 2014 at 03:41:17PM +0100, Victor Kamensky wrote: > > On 4 September 2014 03:10, Stuart Haslam <stuart.haslam@arm.com> wrote: > > > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: > > >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > >> --- > > >> > > >> This creates a new section in the documentaion to group everything > > >> related to buffers. It appears to make things much more acessable > > >> although it needs to have some real description added to the section. > > >> > > > > > > Looks better to me. It also allows more freedom within the > implementation > > > to place things in different header files and still have the > documentation > > > look the same. > > > > If implementations will have freedom to place things in different > headers, > > how one app source will work with all of them? Which header it would > > include? I am not against fancy doxygen syntax, but mapping ODP > > symbol to header file name should be normative part of ODP. Of course, > > such mapping could be supported with set of implementation defined > > headers that are internally included by one that constitute ODP API. > > > > Thanks, > > Victor > > > > This has come up before and I think the intention has always been that > the application should just include odp.h and the implementation can do > whatever it likes with header files beyond that (except perhaps that > they need to be named odp_*.h). > > Personally though, I would rather the file names were fixed, and I did > wonder when writing that comment whether this decision should be > revisited. > > I think in practice this will all work out nicely, but we need to work with it a little, so we can work out the kinks. > -- > Stuart. > >
On 4 September 2014 08:11, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > On 4 September 2014 11:01, Stuart Haslam <stuart.haslam@arm.com> wrote: >> >> On Thu, Sep 04, 2014 at 03:41:17PM +0100, Victor Kamensky wrote: >> > On 4 September 2014 03:10, Stuart Haslam <stuart.haslam@arm.com> wrote: >> > > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: >> > >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> > >> --- >> > >> >> > >> This creates a new section in the documentaion to group everything >> > >> related to buffers. It appears to make things much more acessable >> > >> although it needs to have some real description added to the section. >> > >> >> > > >> > > Looks better to me. It also allows more freedom within the >> > > implementation >> > > to place things in different header files and still have the >> > > documentation >> > > look the same. >> > >> > If implementations will have freedom to place things in different >> > headers, >> > how one app source will work with all of them? Which header it would >> > include? I am not against fancy doxygen syntax, but mapping ODP >> > symbol to header file name should be normative part of ODP. Of course, >> > such mapping could be supported with set of implementation defined >> > headers that are internally included by one that constitute ODP API. >> > >> > Thanks, >> > Victor >> > >> >> This has come up before and I think the intention has always been that >> the application should just include odp.h and the implementation can do >> whatever it likes with header files beyond that (except perhaps that >> they need to be named odp_*.h). >> >> Personally though, I would rather the file names were fixed, and I did >> wonder when writing that comment whether this decision should be >> revisited. >> > I think in practice this will all work out nicely, but we need to work with > it a little, so we can work out the kinks. Not sure which way you argue :). Wrt of Stuart's only one odp.h few remarks: o in linux-generic odp_crypto.h and odp_rwlock.h are not included by odp.h. I guess it is not only me who did not know about one odp.h file rule. I think it should be fixed (i.e odp_crypto.h and odp_rwlock.h) must be added to odp.h. o Examples usage look consistent: most of examples include only odp.h and helper header files. Only place I found is examples/generator includes odp_packet_io.h in addition to odp.h (but it should not, because odp.h does include odp_packet_io.h already) o ODP API Design Guidelines https://www.google.com/url?q=https%3A%2F%2Fdocs.google.com%2Fa%2Flinaro.org%2Fdocument%2Fd%2F15ltgSZolCeN66Xmx9rBSAzpxujia0wj4iPrqb2yzlb8%2Fedit needs section that describes one odp.h header convention, if it is not there yet (I could not find). Thanks, Victor >> >> -- >> Stuart. >> > > > > -- > Mike Holmes > Linaro Technical Manager / Lead > LNG - ODP
On 4 September 2014 11:54, Victor Kamensky <victor.kamensky@linaro.org> wrote: > On 4 September 2014 08:11, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > > > > > On 4 September 2014 11:01, Stuart Haslam <stuart.haslam@arm.com> wrote: > >> > >> On Thu, Sep 04, 2014 at 03:41:17PM +0100, Victor Kamensky wrote: > >> > On 4 September 2014 03:10, Stuart Haslam <stuart.haslam@arm.com> > wrote: > >> > > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: > >> > >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > >> > >> --- > >> > >> > >> > >> This creates a new section in the documentaion to group everything > >> > >> related to buffers. It appears to make things much more acessable > >> > >> although it needs to have some real description added to the > section. > >> > >> > >> > > > >> > > Looks better to me. It also allows more freedom within the > >> > > implementation > >> > > to place things in different header files and still have the > >> > > documentation > >> > > look the same. > >> > > >> > If implementations will have freedom to place things in different > >> > headers, > >> > how one app source will work with all of them? Which header it would > >> > include? I am not against fancy doxygen syntax, but mapping ODP > >> > symbol to header file name should be normative part of ODP. Of course, > >> > such mapping could be supported with set of implementation defined > >> > headers that are internally included by one that constitute ODP API. > >> > > >> > Thanks, > >> > Victor > >> > > >> > >> This has come up before and I think the intention has always been that > >> the application should just include odp.h and the implementation can do > >> whatever it likes with header files beyond that (except perhaps that > >> they need to be named odp_*.h). > >> > >> Personally though, I would rather the file names were fixed, and I did > >> wonder when writing that comment whether this decision should be > >> revisited. > >> > > I think in practice this will all work out nicely, but we need to work > with > > it a little, so we can work out the kinks. > > Not sure which way you argue :). Wrt of Stuart's only one odp.h few > remarks: > > o in linux-generic odp_crypto.h and odp_rwlock.h are not included by > odp.h. I guess it is not only me who did not know about one odp.h file > rule. > I think it should be fixed (i.e odp_crypto.h and odp_rwlock.h) must be > added to odp.h. > > Agree, I had to do this in my global_init fix, but it should just be done. Personally I don't believe in odp.h, I think you should include only what you need. > o Examples usage look consistent: most of examples include only odp.h > and helper header files. Only place I found is examples/generator includes > odp_packet_io.h in addition to odp.h (but it should not, because odp.h > does include odp_packet_io.h already) > Agree > > o ODP API Design Guidelines > > https://www.google.com/url?q=https%3A%2F%2Fdocs.google.com%2Fa%2Flinaro.org%2Fdocument%2Fd%2F15ltgSZolCeN66Xmx9rBSAzpxujia0wj4iPrqb2yzlb8%2Fedit > needs section that describes one odp.h header convention, if it is > not there yet (I could not find). > Agree > > Thanks, > Victor > > >> > >> -- > >> Stuart. > >> > > > > > > > > -- > > Mike Holmes > > Linaro Technical Manager / Lead > > LNG - ODP >
On 09/04/2014 07:05 PM, Mike Holmes wrote: > > > > On 4 September 2014 11:54, Victor Kamensky <victor.kamensky@linaro.org > <mailto:victor.kamensky@linaro.org>> wrote: > > On 4 September 2014 08:11, Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> wrote: > > > > > > > > On 4 September 2014 11:01, Stuart Haslam <stuart.haslam@arm.com > <mailto:stuart.haslam@arm.com>> wrote: > >> > >> On Thu, Sep 04, 2014 at 03:41:17PM +0100, Victor Kamensky wrote: > >> > On 4 September 2014 03:10, Stuart Haslam > <stuart.haslam@arm.com <mailto:stuart.haslam@arm.com>> wrote: > >> > > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: > >> > >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > >> > >> --- > >> > >> > >> > >> This creates a new section in the documentaion to group > everything > >> > >> related to buffers. It appears to make things much more > acessable > >> > >> although it needs to have some real description added to > the section. > >> > >> > >> > > > >> > > Looks better to me. It also allows more freedom within the > >> > > implementation > >> > > to place things in different header files and still have the > >> > > documentation > >> > > look the same. > >> > > >> > If implementations will have freedom to place things in different > >> > headers, > >> > how one app source will work with all of them? Which header it > would > >> > include? I am not against fancy doxygen syntax, but mapping ODP > >> > symbol to header file name should be normative part of ODP. Of > course, > >> > such mapping could be supported with set of implementation defined > >> > headers that are internally included by one that constitute > ODP API. > >> > > >> > Thanks, > >> > Victor > >> > > >> > >> This has come up before and I think the intention has always > been that > >> the application should just include odp.h and the implementation > can do > >> whatever it likes with header files beyond that (except perhaps that > >> they need to be named odp_*.h). > >> > >> Personally though, I would rather the file names were fixed, and > I did > >> wonder when writing that comment whether this decision should be > >> revisited. > >> > > I think in practice this will all work out nicely, but we need to > work with > > it a little, so we can work out the kinks. > > Not sure which way you argue :). Wrt of Stuart's only one odp.h few > remarks: > > o in linux-generic odp_crypto.h and odp_rwlock.h are not included by > odp.h. I guess it is not only me who did not know about one odp.h > file rule. > I think it should be fixed (i.e odp_crypto.h and odp_rwlock.h) must be > added to odp.h. > > Agree, I had to do this in my global_init fix, but it should just be > done. Personally I don't believe in odp.h, I think you should include > only what you need. Mike, which disadvantages common odp.h has? Was there some mail thread there pros and cons of common odp.h were discussed.
I think it obfuscates things, adds overhead to maintainence and to app development. It has to be maintained but it is not strictly necessary, as now, it is already out of date and no one noticed Implementers accidently use it rather than the internal file - overhead to police this. Applications no longer advertise the features they use in their include list, I like that summary at the top of a file, it clearly says that this file deals with crypto for example. When hunting for information I have to go from my app.c to odp.h to find the name of the real include file wanted to look in. I don't like the conceptual coupling, for me coupling is bad when it is not needed for execution - as in this case. The only upside is that apps writers don't wear out their keyboards On 4 September 2014 12:11, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 09/04/2014 07:05 PM, Mike Holmes wrote: > >> >> >> >> On 4 September 2014 11:54, Victor Kamensky <victor.kamensky@linaro.org >> <mailto:victor.kamensky@linaro.org>> wrote: >> >> On 4 September 2014 08:11, Mike Holmes <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> wrote: >> > >> > >> > >> > On 4 September 2014 11:01, Stuart Haslam <stuart.haslam@arm.com >> <mailto:stuart.haslam@arm.com>> wrote: >> >> >> >> On Thu, Sep 04, 2014 at 03:41:17PM +0100, Victor Kamensky wrote: >> >> > On 4 September 2014 03:10, Stuart Haslam >> <stuart.haslam@arm.com <mailto:stuart.haslam@arm.com>> wrote: >> >> > > On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: >> >> > >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> >> >> >> > >> --- >> >> > >> >> >> > >> This creates a new section in the documentaion to group >> everything >> >> > >> related to buffers. It appears to make things much more >> acessable >> >> > >> although it needs to have some real description added to >> the section. >> >> > >> >> >> > > >> >> > > Looks better to me. It also allows more freedom within the >> >> > > implementation >> >> > > to place things in different header files and still have the >> >> > > documentation >> >> > > look the same. >> >> > >> >> > If implementations will have freedom to place things in >> different >> >> > headers, >> >> > how one app source will work with all of them? Which header it >> would >> >> > include? I am not against fancy doxygen syntax, but mapping ODP >> >> > symbol to header file name should be normative part of ODP. Of >> course, >> >> > such mapping could be supported with set of implementation >> defined >> >> > headers that are internally included by one that constitute >> ODP API. >> >> > >> >> > Thanks, >> >> > Victor >> >> > >> >> >> >> This has come up before and I think the intention has always >> been that >> >> the application should just include odp.h and the implementation >> can do >> >> whatever it likes with header files beyond that (except perhaps >> that >> >> they need to be named odp_*.h). >> >> >> >> Personally though, I would rather the file names were fixed, and >> I did >> >> wonder when writing that comment whether this decision should be >> >> revisited. >> >> >> > I think in practice this will all work out nicely, but we need to >> work with >> > it a little, so we can work out the kinks. >> >> Not sure which way you argue :). Wrt of Stuart's only one odp.h few >> remarks: >> >> o in linux-generic odp_crypto.h and odp_rwlock.h are not included by >> odp.h. I guess it is not only me who did not know about one odp.h >> file rule. >> I think it should be fixed (i.e odp_crypto.h and odp_rwlock.h) must be >> added to odp.h. >> >> Agree, I had to do this in my global_init fix, but it should just be >> done. Personally I don't believe in odp.h, I think you should include >> only what you need. >> > > Mike, which disadvantages common odp.h has? > Was there some mail thread there pros and cons of common odp.h were > discussed. >
On 09/04/2014 07:21 PM, Mike Holmes wrote: > I think it obfuscates things, adds overhead to maintainence and to app > development. > > It has to be maintained but it is not strictly necessary, as now, it is > already out of date and no one noticed > Implementers accidently use it rather than the internal file - overhead > to police this. > Applications no longer advertise the features they use in their include > list, I like that summary at the top of a file, it clearly says that > this file deals with crypto for example. > When hunting for information I have to go from my app.c to odp.h to find > the name of the real include file wanted to look in. > I don't like the conceptual coupling, for me coupling is bad when it is > not needed for execution - as in this case. > > The only upside is that apps writers don't wear out their keyboards I partially agree, but having separate headers exposed to application makes header names a part of specification. So if let's say in future we would like to reorganize headers and move all type definitions to a separate file, then all applications have to be updated.
On 4 September 2014 12:56, Taras Kondratiuk <taras.kondratiuk@linaro.org> wrote: > On 09/04/2014 07:21 PM, Mike Holmes wrote: > >> I think it obfuscates things, adds overhead to maintainence and to app >> development. >> >> It has to be maintained but it is not strictly necessary, as now, it is >> already out of date and no one noticed >> Implementers accidently use it rather than the internal file - overhead >> to police this. >> Applications no longer advertise the features they use in their include >> list, I like that summary at the top of a file, it clearly says that >> this file deals with crypto for example. >> When hunting for information I have to go from my app.c to odp.h to find >> the name of the real include file wanted to look in. >> I don't like the conceptual coupling, for me coupling is bad when it is >> not needed for execution - as in this case. >> >> The only upside is that apps writers don't wear out their keyboards >> > > I partially agree, but having separate headers exposed to application > makes header names a part of specification. So if let's say in future > we would like to reorganize headers and move all type definitions to a > separate file, then all applications have to be updated. > I would pay that price and call it ODP 3.0, you can't always ensure backwards compatibility or you stagnate.
I experimented with defgroup and ingroup. Using ingroup means you have to tag each thing individually, trying to combine it with "@{" was not successful so I think addtogroup is best, it will concatenate large blocks of API. In future we can go mark each thing with @ingroup <block name> if we need that atomicity defgroup can replace the major section addtogroup without a problem. So having one defgroup with other regions marked with addtogroup appears the way forward to me. Mike On 4 September 2014 11:07, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > On 4 September 2014 06:10, Stuart Haslam <stuart.haslam@arm.com> wrote: > >> On Wed, Sep 03, 2014 at 03:18:11AM +0100, Mike Holmes wrote: >> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> > --- >> > >> > This creates a new section in the documentaion to group everything >> > related to buffers. It appears to make things much more acessable >> > although it needs to have some real description added to the section. >> > >> >> Looks better to me. It also allows more freedom within the implementation >> to place things in different header files and still have the documentation >> look the same. >> >> Small nit, I think it would be better to use defgroup and ingroup, rather >> than addtogroup, to avoid any confusion about where the documentation for >> that group should go (what happens if you document multiple addtogroups?). >> > > > We can make it explicit, I was thinking that it was easier to manage with > the looser association. > multiple add groups just concatenate without a warning. > > > > >> -- >> Stuart. >> >> > platform/linux-generic/include/api/odp_buffer.h | 6 +++++- >> > platform/linux-generic/include/api/odp_buffer_pool.h | 6 ++++++ >> > 2 files changed, 11 insertions(+), 1 deletion(-) >> > >> > diff --git a/platform/linux-generic/include/api/odp_buffer.h >> b/platform/linux-generic/include/api/odp_buffer.h >> > index d8577fd..4f348e4 100644 >> > --- a/platform/linux-generic/include/api/odp_buffer.h >> > +++ b/platform/linux-generic/include/api/odp_buffer.h >> > @@ -21,7 +21,10 @@ extern "C" { >> > >> > #include <odp_std_types.h> >> > >> > - >> > +/** >> > + * @addtogroup buffer >> > + * @{ >> > + */ >> > >> > /** >> > * ODP buffer >> > @@ -83,6 +86,7 @@ int odp_buffer_is_valid(odp_buffer_t buf); >> > */ >> > void odp_buffer_print(odp_buffer_t buf); >> > >> > +/** @} */ >> > >> > #ifdef __cplusplus >> > } >> > diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h >> b/platform/linux-generic/include/api/odp_buffer_pool.h >> > index fe88898..dd0ac43 100644 >> > --- a/platform/linux-generic/include/api/odp_buffer_pool.h >> > +++ b/platform/linux-generic/include/api/odp_buffer_pool.h >> > @@ -23,6 +23,11 @@ extern "C" { >> > #include <odp_std_types.h> >> > #include <odp_buffer.h> >> > >> > +/** >> > + * @addtogroup buffer ODP Buffers and buffer Pools >> > + * @{ >> > + */ >> > + >> > /** Maximum queue name lenght in chars */ >> > #define ODP_BUFFER_POOL_NAME_LEN 32 >> > >> > @@ -99,6 +104,7 @@ void odp_buffer_free(odp_buffer_t buf); >> > */ >> > odp_buffer_pool_t odp_buffer_pool(odp_buffer_t buf); >> > >> > +/** @} */ >> > >> > #ifdef __cplusplus >> > } >> > -- >> > 1.9.1 >> > >> >> >> > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP >
On Thu, Sep 04, 2014 at 10:34:24PM +0100, Mike Holmes wrote: > I experimented with defgroup and ingroup. > > Using ingroup means you have to tag each thing individually, trying to combine it with "@{" was not successful so I think addtogroup is best, it will concatenate large blocks of API. > In future we can go mark each thing with @ingroup <block name> if we need that atomicity > defgroup can replace the major section addtogroup without a problem. > > So having one defgroup with other regions marked with addtogroup appears the way forward to me. > I agree, I didn't know about that restriction with ingroup. -- Stuart.
On 09/04/2014 08:16 PM, Mike Holmes wrote: > > > > On 4 September 2014 12:56, Taras Kondratiuk <taras.kondratiuk@linaro.org > <mailto:taras.kondratiuk@linaro.org>> wrote: > > On 09/04/2014 07:21 PM, Mike Holmes wrote: > > I think it obfuscates things, adds overhead to maintainence and > to app > development. > > It has to be maintained but it is not strictly necessary, as > now, it is > already out of date and no one noticed > Implementers accidently use it rather than the internal file - > overhead > to police this. > Applications no longer advertise the features they use in their > include > list, I like that summary at the top of a file, it clearly says that > this file deals with crypto for example. > When hunting for information I have to go from my app.c to odp.h > to find > the name of the real include file wanted to look in. > I don't like the conceptual coupling, for me coupling is bad > when it is > not needed for execution - as in this case. > > The only upside is that apps writers don't wear out their keyboards > > > I partially agree, but having separate headers exposed to application > makes header names a part of specification. So if let's say in future > we would like to reorganize headers and move all type definitions to a > separate file, then all applications have to be updated. > > > I would pay that price and call it ODP 3.0, you can't always ensure > backwards compatibility or you stagnate. Ok, then IMO before ODP v1.0 we have to revise header names and its content. Decide which of them are public and add this to specification. IMO currently we have too many public headers. For example is odp_packet_flags.h need to be included by application or it can be implicitly always included by odp_packet.h?
diff --git a/platform/linux-generic/include/api/odp_buffer.h b/platform/linux-generic/include/api/odp_buffer.h index d8577fd..4f348e4 100644 --- a/platform/linux-generic/include/api/odp_buffer.h +++ b/platform/linux-generic/include/api/odp_buffer.h @@ -21,7 +21,10 @@ extern "C" { #include <odp_std_types.h> - +/** + * @addtogroup buffer + * @{ + */ /** * ODP buffer @@ -83,6 +86,7 @@ int odp_buffer_is_valid(odp_buffer_t buf); */ void odp_buffer_print(odp_buffer_t buf); +/** @} */ #ifdef __cplusplus } diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h b/platform/linux-generic/include/api/odp_buffer_pool.h index fe88898..dd0ac43 100644 --- a/platform/linux-generic/include/api/odp_buffer_pool.h +++ b/platform/linux-generic/include/api/odp_buffer_pool.h @@ -23,6 +23,11 @@ extern "C" { #include <odp_std_types.h> #include <odp_buffer.h> +/** + * @addtogroup buffer ODP Buffers and buffer Pools + * @{ + */ + /** Maximum queue name lenght in chars */ #define ODP_BUFFER_POOL_NAME_LEN 32 @@ -99,6 +104,7 @@ void odp_buffer_free(odp_buffer_t buf); */ odp_buffer_pool_t odp_buffer_pool(odp_buffer_t buf); +/** @} */ #ifdef __cplusplus }
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- This creates a new section in the documentaion to group everything related to buffers. It appears to make things much more acessable although it needs to have some real description added to the section. platform/linux-generic/include/api/odp_buffer.h | 6 +++++- platform/linux-generic/include/api/odp_buffer_pool.h | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-)