[Freeipa-devel] [PATCH 0224-0225] Add function attributes warn_unused_result and nonnull and add missing CHECK()s to string operations
Lukas Slebodnik
lslebodn at redhat.com
Fri Feb 21 18:14:37 UTC 2014
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>
>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.
- 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
More information about the Freeipa-devel
mailing list