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

[Petr Spacek]

On 21.2.2014 19:14, Lukas Slebodnik wrote:
> On (21/02/14 16:12), 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! :-)
>>
>> --
>> Petr^2 Spacek
>
>>From 063f776fc083c1fa26419a1ea63df98b9953826f Mon Sep 17 00:00:00 2001
>> From: Petr Spacek <pspacek at redhat.com>
>> Date: Fri, 21 Feb 2014 15:21:36 +0100
>> Subject: [PATCH] Add missing CHECK()s to string operations.
>>
>> Signed-off-by: Petr Spacek <pspacek at redhat.com>
>> ---
>> src/ldap_helper.c |  4 ++--
>> src/str.c         |  4 ++--
>> src/str.h         | 28 ++++++++++++++--------------
>> src/util.h        |  2 ++
>> 4 files changed, 20 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
>> index b0dd3391f4dca88992ac7869b34d943a381d51be..be37ce575c0965856afabcb59c5eba949ad902fd 100644
>> --- a/src/ldap_helper.c
>> +++ b/src/ldap_helper.c
>> @@ -1772,7 +1772,7 @@ ldap_replace_serial(ldap_instance_t *inst, dns_name_t *zone,
>>
>> 	REQUIRE(inst != NULL);
>>
>> -	str_new(inst->mctx, &dn);
>> +	CHECK(str_new(inst->mctx, &dn));
>> 	CHECK(dnsname_to_dn(inst->zone_register, zone, dn));
>>
>> 	change.mod_op = LDAP_MOD_REPLACE;
>> @@ -2405,7 +2405,7 @@ ldap_query(ldap_instance_t *ldap_inst, ldap_connection_t *ldap_conn,
>> 		CHECK(ldap_pool_getconnection(ldap_inst->pool, &ldap_conn));
>>
>> 	va_start(ap, filter);
>> -	str_vsprintf(ldap_qresult->query_string, filter, ap);
>> +	CHECK(str_vsprintf(ldap_qresult->query_string, filter, ap));
>> 	va_end(ap);
>      ^^^^^^^^^^
> va_end have to be called every time.
> It would be better to move check after va_end(ap)
>          va_start(ap, filter);
>          result = str_vsprintf(ldap_qresult->query_string, filter, ap);
>          va_end(ap);
>          CHECK(result);
>
> <snip>

Fixed and pushed to master: 960e00f4dee9a4be82c61a968e7b31aa863638cd

>>From 50cb2a22cad24463145b8e18582d13fc20dc8011 Mon Sep 17 00:00:00 2001
>> From: Petr Spacek <pspacek at redhat.com>
>> Date: Fri, 21 Feb 2014 15:58:19 +0100
>> Subject: [PATCH] Add function attributes warn_unused_result and nonnull where
>> appropriate.
>>
>> Signed-off-by: Petr Spacek <pspacek at redhat.com>
>> ---
>> src/acl.c           |  22 ++++----
>> src/acl.h           |   6 +-
>> src/fs.h            |   4 +-
>> src/fwd_register.h  |  10 ++--
>> src/krb5_helper.c   |   2 +-
>> src/krb5_helper.h   |   2 +-
>> src/ldap_convert.c  |   8 +--
>> src/ldap_convert.h  |  10 ++--
>> src/ldap_entry.c    |   2 +-
>> src/ldap_entry.h    |  26 ++++-----
>> src/ldap_helper.c   | 156 ++++++++++++++++++++++++++--------------------------
>> src/ldap_helper.h   |   2 +-
>> src/rbt_helper.c    |   2 +-
>> src/rbt_helper.h    |   4 +-
>> src/rdlist.c        |   4 +-
>> src/rdlist.h        |   8 +--
>> src/semaphore.h     |   4 +-
>> src/settings.c      |   4 +-
>> src/settings.h      |  18 +++---
>> src/syncrepl.c      |   2 +-
>> src/syncrepl.h      |  14 ++---
>> src/zone_manager.c  |   4 +-
>> src/zone_manager.h  |   6 +-
>> src/zone_register.h |  18 +++---
>> 24 files changed, 169 insertions(+), 169 deletions(-)
>>
>
> Patch works well. I did a small test with a function find_db_instance.

Pushed to master: 7e4323eacb74ad6a5658cc256fc4c347abc01ddc

>
> -       result = find_db_instance(name, &db_inst);
> +       find_db_instance(name, &db_inst);
>
>    CC       ldap_la-zone_manager.lo
> ../src/zone_manager.c:126:2: error: ignoring return value of function declared with
>        warn_unused_result attribute [-Werror,-Wunused-result]
>          find_db_instance(name, &db_inst);
>          ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~
> 1 error generated.
>
> Attribute "__attribute__((warn_unused_result))" is not generated
> if macro __GNUC__ is not defined. (make CFLAGS+="-U__GNUC__")
>
> Some lines are longer than 80 columns, but I am not very familiar with
> your coding style. Otherwise 2nd patch looks good.
>
> LS

Thank you very much for your time!

-- 
Petr^2 Spacek