On 13 January 2015 at 04:29, Ola Liljedahl <[email protected]> wrote:
> On 13 January 2015 at 09:45, Savolainen, Petri (NSN - FI/Espoo) > <[email protected]> wrote: > >> +void odp_coremask_from_str(const char *str, odp_coremask_t *mask) > >> + ODP_NONNULL(1, 2); > > > > This is kind of ugly at API level. I was thinking that > __attribute__((__nonnull_)) would work in (function) implementation. Is > that possible? We can add easily a number of attributes into implementation > to aid static checking, but API should be kept clean. > > > > Also according to this blog (from 2010), GCC has done "optimizations" > based on the non-null tagging and e.g. removed if(ptr == NULL) checks from > the function implementation. > > > > > http://l.longi.li/blog/2010/04/19/gcc-s-attribute-nonnull-not-helpful-at-all/ > If the state of warnings and code generation is still as described in > this post, then attribute non-null is useless and counter-productive. > Interesting I need to read that in detail - posted in 2010 though so I wonder if it is still relevant > > > > > > Could compiler be clever enough to spot non-null property from function > implementation (from pointer check against NULL)? That code could be > #ifdef'd there for static analysis / unit testing. > Just because the code checks a parameter for non-null doesn't mean the > compiler can understand this is an invalid parameter value. How can > the code tell the *compiler* that based on some value checking, a > parameter is invalid? There are no compiler directives (AFAIK) to > invoke in such a situation and calling e.g. abort() is legal and could > be the desired outcome from the caller's point of view. > > > > > > -Petri > > > > > > > >> -----Original Message----- > >> From: [email protected] [mailto:lng-odp- > >> [email protected]] On Behalf Of ext Mike Holmes > >> Sent: Monday, January 12, 2015 11:44 PM > >> To: [email protected] > >> Subject: [lng-odp] [PATCH 2/2] linux-genric: odp_core_mask add > ODP_NONNULL > >> > >> Allow the complier to check for NULL args. > >> > >> Signed-off-by: Mike Holmes <[email protected]> > >> --- > >> platform/linux-generic/include/api/odp_coremask.h | 18 > +++++++++++------- > >> 1 file changed, 11 insertions(+), 7 deletions(-) > >> > >> diff --git a/platform/linux-generic/include/api/odp_coremask.h > >> b/platform/linux-generic/include/api/odp_coremask.h > >> index c9331fd..b52f977 100644 > >> --- a/platform/linux-generic/include/api/odp_coremask.h > >> +++ b/platform/linux-generic/include/api/odp_coremask.h > >> @@ -21,6 +21,7 @@ extern "C" { > >> > >> > >> #include <odp_std_types.h> > >> +#include <odp_hints.h> > >> > >> /** @addtogroup odp_scheduler > >> * Core mask operations. > >> @@ -51,7 +52,8 @@ typedef struct odp_coremask_t { > >> * > >> * @note Supports currently only core indexes upto 63 > >> */ > >> -void odp_coremask_from_str(const char *str, odp_coremask_t *mask); > >> +void odp_coremask_from_str(const char *str, odp_coremask_t *mask) > >> + ODP_NONNULL(1, 2); > >> > >> /** > >> * Write core mask as a string of hexadecimal digits > >> @@ -64,7 +66,8 @@ void odp_coremask_from_str(const char *str, > >> odp_coremask_t *mask); > >> * > >> * @note Supports currently only core indexes upto 63 > >> */ > >> -void odp_coremask_to_str(char *str, int len, const odp_coremask_t > *mask); > >> +void odp_coremask_to_str(char *str, int len, const odp_coremask_t > *mask) > >> + ODP_NONNULL(1, 3); > >> > >> > >> /** > >> @@ -87,7 +90,8 @@ void odp_coremask_to_str(char *str, int len, const > >> odp_coremask_t *mask); > >> * > >> * @note Supports currently only core indexes upto 63 > >> */ > >> -void odp_coremask_from_u64(const uint64_t *u64, int num, odp_coremask_t > >> *mask); > >> +void odp_coremask_from_u64(const uint64_t *u64, int num, odp_coremask_t > >> *mask) > >> + ODP_NONNULL(1, 3); > >> > >> /** > >> * Clear entire mask > >> @@ -103,14 +107,14 @@ static inline void > odp_coremask_zero(odp_coremask_t > >> *mask) > >> * @param core Core number > >> * @param mask add core number in core mask > >> */ > >> -void odp_coremask_set(int core, odp_coremask_t *mask); > >> +void odp_coremask_set(int core, odp_coremask_t *mask) ODP_NONNULL(2); > >> > >> /** > >> * Remove core from mask > >> * @param core Core number > >> * @param mask clear core number from core mask > >> */ > >> -void odp_coremask_clr(int core, odp_coremask_t *mask); > >> +void odp_coremask_clr(int core, odp_coremask_t *mask) ODP_NONNULL(2); > >> > >> /** > >> * Test if core is a member of mask > >> @@ -118,14 +122,14 @@ void odp_coremask_clr(int core, odp_coremask_t > >> *mask); > >> * @param mask Core mask to check if core num set or not > >> * @return non-zero if set otherwise 0 > >> */ > >> -int odp_coremask_isset(int core, const odp_coremask_t *mask); > >> +int odp_coremask_isset(int core, const odp_coremask_t *mask) > >> ODP_NONNULL(2); > >> > >> /** > >> * Count number of cores in mask > >> * @param mask Core mask > >> * @return coremask count > >> */ > >> -int odp_coremask_count(const odp_coremask_t *mask); > >> +int odp_coremask_count(const odp_coremask_t *mask) ODP_NONNULL(1); > >> > >> > >> > >> -- > >> 2.1.0 > >> > >> > >> _______________________________________________ > >> lng-odp mailing list > >> [email protected] > >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > > lng-odp mailing list > > [email protected] > > http://lists.linaro.org/mailman/listinfo/lng-odp > -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
