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.

>
>
> 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

_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to