| Message ID | 1409259222-957-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
> -----Original Message----- > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > Sent: Thursday, August 28, 2014 11:54 PM > To: lng-odp@lists.linaro.org > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document odp_shm_reserve > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > platform/linux-generic/include/api/odp_shared_memory.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > b/platform/linux-generic/include/api/odp_shared_memory.h > index 8ac8847..d75b179 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -21,7 +21,7 @@ extern "C" { > > #include <odp_std_types.h> > > -/** Maximum shared memory block name lenght in chars */ > +/** Maximum shared memory block name length in chars */ > #define ODP_SHM_NAME_LEN 32 > > > @@ -33,6 +33,9 @@ extern "C" { > * @param align Block alignment in bytes > * > * @return Pointer to the reserved block, or NULL > + * > + * @note The same block name cannot be reused. > + * @note There is no way to unreserve a named block I think there will be a odp_shm_release call. So I'd not put that last note there...otherwise half of the current API should be noted about missing features. -Petri > */ > void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align); > > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 08/29/2014 12:53 AM, Mike Holmes wrote: > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > platform/linux-generic/include/api/odp_shared_memory.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h b/platform/linux-generic/include/api/odp_shared_memory.h > index 8ac8847..d75b179 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -21,7 +21,7 @@ extern "C" { > > #include <odp_std_types.h> > > -/** Maximum shared memory block name lenght in chars */ > +/** Maximum shared memory block name length in chars */ > #define ODP_SHM_NAME_LEN 32 > > > @@ -33,6 +33,9 @@ extern "C" { > * @param align Block alignment in bytes > * > * @return Pointer to the reserved block, or NULL > + * > + * @note The same block name cannot be reused. > + * @note There is no way to unreserve a named block First note with dot, second note without. > */ > void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align); >
On 29 August 2014 04:05, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Thursday, August 28, 2014 11:54 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document odp_shm_reserve > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > platform/linux-generic/include/api/odp_shared_memory.h | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > > b/platform/linux-generic/include/api/odp_shared_memory.h > > index 8ac8847..d75b179 100644 > > --- a/platform/linux-generic/include/api/odp_shared_memory.h > > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > > @@ -21,7 +21,7 @@ extern "C" { > > > > #include <odp_std_types.h> > > > > -/** Maximum shared memory block name lenght in chars */ > > +/** Maximum shared memory block name length in chars */ > > #define ODP_SHM_NAME_LEN 32 > > > > > > @@ -33,6 +33,9 @@ extern "C" { > > * @param align Block alignment in bytes > > * > > * @return Pointer to the reserved block, or NULL > > + * > > + * @note The same block name cannot be reused. > > + * @note There is no way to unreserve a named block > > I think there will be a odp_shm_release call. So I'd not put that last > note there...otherwise half of the current API should be noted about > missing features. > The problem I face is that this is the API as it stands, and we don't have any roadmap that says the symmetry will be added by 1.0, we need one. It comes into focus right now that the unit tests are running into it.The test cases either have to prove an API that does not exist or up date the docs to match reality and test against that. I feel the best approach rather than add todo allover is to actually document reality, prove it with the unit tests, and then update the code/docs/unit tests as we make progress. > > -Petri > > > */ > > void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align); > > > > -- > > 1.9.1 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp >
We definitely need the closure APIs to be part of v1.0 for completeness. For now noting their absence as bugs is a good way of reminding us of that fact. Bill On Fri, Aug 29, 2014 at 6:34 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > On 29 August 2014 04:05, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > >> >> >> > -----Original Message----- >> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >> > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes >> > Sent: Thursday, August 28, 2014 11:54 PM >> > To: lng-odp@lists.linaro.org >> > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document odp_shm_reserve >> > >> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> > --- >> > platform/linux-generic/include/api/odp_shared_memory.h | 5 ++++- >> > 1 file changed, 4 insertions(+), 1 deletion(-) >> > >> > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h >> > b/platform/linux-generic/include/api/odp_shared_memory.h >> > index 8ac8847..d75b179 100644 >> > --- a/platform/linux-generic/include/api/odp_shared_memory.h >> > +++ b/platform/linux-generic/include/api/odp_shared_memory.h >> > @@ -21,7 +21,7 @@ extern "C" { >> > >> > #include <odp_std_types.h> >> > >> > -/** Maximum shared memory block name lenght in chars */ >> > +/** Maximum shared memory block name length in chars */ >> > #define ODP_SHM_NAME_LEN 32 >> > >> > >> > @@ -33,6 +33,9 @@ extern "C" { >> > * @param align Block alignment in bytes >> > * >> > * @return Pointer to the reserved block, or NULL >> > + * >> > + * @note The same block name cannot be reused. >> > + * @note There is no way to unreserve a named block >> >> I think there will be a odp_shm_release call. So I'd not put that last >> note there...otherwise half of the current API should be noted about >> missing features. >> > The problem I face is that this is the API as it stands, and we don't > have any roadmap that says the symmetry will be added by 1.0, we need one. > > It comes into focus right now that the unit tests are running into it.The > test cases either have to prove an API that does not exist or up date the > docs to match reality and test against that. > > I feel the best approach rather than add todo allover is to actually > document reality, prove it with the unit tests, and then update the > code/docs/unit tests as we make progress. > >> >> -Petri >> >> > */ >> > void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align); >> > >> > -- >> > 1.9.1 >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
Still book keeping should be done in a single place. It’s confusing if TODO/missing feature lists are spread over doxygen comments, bugzilla bug reports, cards, … I’d use doxygen only for documenting the existing code, and keep list missing features somewhere else. E.g. in this case, when someone adds the missing function (eventually), it’s easy to forget to remove the comment from the reserve function (or N other places) -Petri From: ext Bill Fischofer [mailto:bill.fischofer@linaro.org] Sent: Friday, August 29, 2014 3:19 PM To: Mike Holmes Cc: Savolainen, Petri (NSN - FI/Espoo); lng-odp@lists.linaro.org Subject: Re: [lng-odp] [PATCH] odp_shared_memory.h: Document odp_shm_reserve We definitely need the closure APIs to be part of v1.0 for completeness. For now noting their absence as bugs is a good way of reminding us of that fact. Bill On Fri, Aug 29, 2014 at 6:34 AM, Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> wrote: On 29 August 2014 04:05, Savolainen, Petri (NSN - FI/Espoo) <petri.savolainen@nsn.com<mailto:petri.savolainen@nsn.com>> wrote: > -----Original Message----- > From: lng-odp-bounces@lists.linaro.org<mailto:lng-odp-bounces@lists.linaro.org> [mailto:lng-odp-<mailto:lng-odp-> > bounces@lists.linaro.org<mailto:bounces@lists.linaro.org>] On Behalf Of ext Mike Holmes > Sent: Thursday, August 28, 2014 11:54 PM > To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document odp_shm_reserve > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>> > --- > platform/linux-generic/include/api/odp_shared_memory.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > b/platform/linux-generic/include/api/odp_shared_memory.h > index 8ac8847..d75b179 100644 > --- a/platform/linux-generic/include/api/odp_shared_memory.h > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > @@ -21,7 +21,7 @@ extern "C" { > > #include <odp_std_types.h> > > -/** Maximum shared memory block name lenght in chars */ > +/** Maximum shared memory block name length in chars */ > #define ODP_SHM_NAME_LEN 32 > > > @@ -33,6 +33,9 @@ extern "C" { > * @param align Block alignment in bytes > * > * @return Pointer to the reserved block, or NULL > + * > + * @note The same block name cannot be reused. > + * @note There is no way to unreserve a named block I think there will be a odp_shm_release call. So I'd not put that last note there...otherwise half of the current API should be noted about missing features. The problem I face is that this is the API as it stands, and we don't have any roadmap that says the symmetry will be added by 1.0, we need one. It comes into focus right now that the unit tests are running into it.The test cases either have to prove an API that does not exist or up date the docs to match reality and test against that. I feel the best approach rather than add todo allover is to actually document reality, prove it with the unit tests, and then update the code/docs/unit tests as we make progress. -Petri > */ > void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align); > > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp -- Mike Holmes Linaro Technical Manager / Lead LNG - ODP _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org> http://lists.linaro.org/mailman/listinfo/lng-odp
I think this discussion is really good, it highlights that we don't have a list of what is missing and what people know needs to be done "todo". *If a feature has never existed it needs to be a new work card.* *If the code exists and it fails to work fully or is not well documented I think it is a bug that needs a patch, or if it is simple just submit a patch.* *All todos should be bugs before the code is accepted, the debug tag/id should be in the todo* In this case I think it is important that the actual checked in code has a significant flaw that a user should know about, it should be a bug to add the functionality which might take a while to be solved. If LNG pick up solving it it will become a CARD and eventually it will be a patch either way. On 29 August 2014 08:49, Savolainen, Petri (NSN - FI/Espoo) < petri.savolainen@nsn.com> wrote: > Still book keeping should be done in a single place. It’s confusing if > TODO/missing feature lists are spread over doxygen comments, bugzilla bug > reports, cards, … I’d use doxygen only for documenting the existing code, > and keep list missing features somewhere else. > > > > E.g. in this case, when someone adds the missing function (eventually), > it’s easy to forget to remove the comment from the reserve function (or N > other places) > In my opinion worries about correctly changing the code in the future is not a good reason not to fix what does exist now. Options: - In this case we can give the heads up with a simple patch (this one as is) - Convert it to a todo that lists the bug ID. - THE BEST WAY - is in this thread we define the new API and this patch can add it with a dummy implementation that returns unsupported. But I am ok with a todo - is that ok with you Petri? > > > -Petri > > > > > > *From:* ext Bill Fischofer [mailto:bill.fischofer@linaro.org] > *Sent:* Friday, August 29, 2014 3:19 PM > *To:* Mike Holmes > *Cc:* Savolainen, Petri (NSN - FI/Espoo); lng-odp@lists.linaro.org > *Subject:* Re: [lng-odp] [PATCH] odp_shared_memory.h: Document > odp_shm_reserve > > > > We definitely need the closure APIs to be part of v1.0 for completeness. > For now noting their absence as bugs is a good way of reminding us of that > fact. > > > > Bill > > > > On Fri, Aug 29, 2014 at 6:34 AM, Mike Holmes <mike.holmes@linaro.org> > wrote: > > > > > > On 29 August 2014 04:05, Savolainen, Petri (NSN - FI/Espoo) < > petri.savolainen@nsn.com> wrote: > > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Thursday, August 28, 2014 11:54 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH] odp_shared_memory.h: Document odp_shm_reserve > > > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > platform/linux-generic/include/api/odp_shared_memory.h | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/platform/linux-generic/include/api/odp_shared_memory.h > > b/platform/linux-generic/include/api/odp_shared_memory.h > > index 8ac8847..d75b179 100644 > > --- a/platform/linux-generic/include/api/odp_shared_memory.h > > +++ b/platform/linux-generic/include/api/odp_shared_memory.h > > @@ -21,7 +21,7 @@ extern "C" { > > > > #include <odp_std_types.h> > > > > -/** Maximum shared memory block name lenght in chars */ > > +/** Maximum shared memory block name length in chars */ > > #define ODP_SHM_NAME_LEN 32 > > > > > > @@ -33,6 +33,9 @@ extern "C" { > > * @param align Block alignment in bytes > > * > > * @return Pointer to the reserved block, or NULL > > + * > > + * @note The same block name cannot be reused. > > + * @note There is no way to unreserve a named block > > I think there will be a odp_shm_release call. So I'd not put that last > note there...otherwise half of the current API should be noted about > missing features. > > The problem I face is that this is the API as it stands, and we don't > have any roadmap that says the symmetry will be added by 1.0, we need one. > > > > It comes into focus right now that the unit tests are running into it.The > test cases either have to prove an API that does not exist or up date the > docs to match reality and test against that. > > > > I feel the best approach rather than add todo allover is to actually > document reality, prove it with the unit tests, and then update the > code/docs/unit tests as we make progress. > > > -Petri > > > > */ > > void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align); > > > > -- > > 1.9.1 > > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > -- > > *Mike Holmes* > > Linaro Technical Manager / Lead > > LNG - ODP > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > > >
diff --git a/platform/linux-generic/include/api/odp_shared_memory.h b/platform/linux-generic/include/api/odp_shared_memory.h index 8ac8847..d75b179 100644 --- a/platform/linux-generic/include/api/odp_shared_memory.h +++ b/platform/linux-generic/include/api/odp_shared_memory.h @@ -21,7 +21,7 @@ extern "C" { #include <odp_std_types.h> -/** Maximum shared memory block name lenght in chars */ +/** Maximum shared memory block name length in chars */ #define ODP_SHM_NAME_LEN 32 @@ -33,6 +33,9 @@ extern "C" { * @param align Block alignment in bytes * * @return Pointer to the reserved block, or NULL + * + * @note The same block name cannot be reused. + * @note There is no way to unreserve a named block */ void *odp_shm_reserve(const char *name, uint64_t size, uint64_t align);
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- platform/linux-generic/include/api/odp_shared_memory.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)