[Freeipa-devel] [PATCH 0224-0225] Add function attributes warn_unused_result and nonnull and add missing CHECK()s to string operations

[Lukas Slebodnik]

On (21/02/14 10:20), Nathaniel McCallum wrote:
>On Fri, 2014-02-21 at 16:17 +0100, Petr Spacek wrote:
>> On 21.2.2014 16:16, Nathaniel McCallum wrote:
>> > On Fri, 2014-02-21 at 16:12 +0100, Petr Spacek wrote:
>> >> Hello,
>> >>
>> >> Add function attributes warn_unused_result and nonnull
>> >> where appropriate and add missing CHECK()s to string operations.
>> >>
>> >> Lukas, thanks for catching the missing CHECK() around str_new().
>> >>
>> >> As a reward, you can review attached patches.
>> >>
>> >> Have fun! :-)
>> >
>> > NACK
>> >
>> > Adding attributes to a function definition is a no-op and can create
>> > potential confusion should the definitions and declarations differ.
>> >
>> > I would strongly prefer that they are only used in function
>> > declarations.
>> >
>> > Nathaniel
>> 
>> Sorry, but you are not right.
>> 
>> Attributes work perfectly fine for static functions without previous declaration.
>
>Gah. You're right. I had assumed the header changes were duplicates of
>the static functions. Please disregard.
>
1. It does not make sense to have declaration of static function in header file
2. Sometimes, it's better to test patches instead of sending useless reply.

For your information:

You need to use attribute after declaration.
    isc_result_t
    manager_get_ldap_instance(const char *name,
                              ldap_instance_t **ldap_inst) ATTR_NONNULLS;


In case of static function, you need to use attribute before name of function.
In other case, it would be no-op. This is a small difference between using
attributes with function declaration and function definition.

    static isc_result_t ATTR_NONNULLS
    setting_get(const char *const name, const setting_type_t type,
                        const settings_set_t *const set, void *target)
    {
            /* body */
            return 0;
    }

LS