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 <[email protected]> >Date: Fri, 21 Feb 2014 15:21:36 +0100 >Subject: [PATCH] Add missing CHECK()s to string operations. > >Signed-off-by: Petr Spacek <[email protected]> >--- > 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 <[email protected]> >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 <[email protected]> >--- > 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 _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
