- exit log routine is annotated as noreturn function.

- free memory leaks in config strlist and str2list insert functions.
- do not move unused argv variable after getopt.
- Remove unused if clause in testcode.

git-svn-id: file:///svn/unbound/trunk@4896 be551aaa-1e26-0410-a405-d3ace91eadb9

diff --git a/config.h.in b/config.h.in
index 04356f334edf2d7762a6a83edc75a248f8b88e70..d187e00105069cf26593e41366ffbf6361ac4018 100644 (file)
--- a/config.h.in
+++ b/config.h.in
@@ -1,5 +1,8 @@
 /* config.h.in.  Generated from configure.ac by autoheader.  */
 
+/* apply the noreturn attribute to a function that exits the program */
+#undef ATTR_NORETURN
+
 /* Directory to chroot to */
 #undef CHROOT_DIR
 
@@ -45,6 +48,9 @@
 /* Whether the C compiler accepts the "format" attribute */
 #undef HAVE_ATTR_FORMAT
 
+/* Whether the C compiler accepts the "noreturn" attribute */
+#undef HAVE_ATTR_NORETURN
+
 /* Whether the C compiler accepts the "unused" attribute */
 #undef HAVE_ATTR_UNUSED
 

diff --git a/configure b/configure
index 7df1206d67d040fed45724b1365e6fb7bc407475..6666e45469b4eadf3f14644585c4e640ae4e3b86 100755 (executable)
--- a/configure
+++ b/configure
@@ -6269,6 +6269,51 @@ $as_echo "#define HAVE_ATTR_WEAK 1" >>confdefs.h
 fi
 
 
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the C compiler (${CC-cc}) accepts the \"noreturn\" attribute" >&5
+$as_echo_n "checking whether the C compiler (${CC-cc}) accepts the \"noreturn\" attribute... " >&6; }
+if ${ac_cv_c_noreturn_attribute+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_cv_c_noreturn_attribute=no
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+ #include <stdio.h>
+__attribute__((noreturn)) void f(int x) { printf("%d", x); }
+
+int
+main ()
+{
+
+   f(1);
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  ac_cv_c_noreturn_attribute="yes"
+else
+  ac_cv_c_noreturn_attribute="no"
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_c_noreturn_attribute" >&5
+$as_echo "$ac_cv_c_noreturn_attribute" >&6; }
+if test $ac_cv_c_noreturn_attribute = yes; then
+
+$as_echo "#define HAVE_ATTR_NORETURN 1" >>confdefs.h
+
+
+$as_echo "#define ATTR_NORETURN __attribute__((__noreturn__))" >>confdefs.h
+
+fi
+
+
 if test "$srcdir" != "."; then
        CPPFLAGS="$CPPFLAGS -I$srcdir"
 fi

diff --git a/configure.ac b/configure.ac
index 44e65cdd96ae229deefc7bbc18dd8c430835e56e..449cf32664645535f3d9ff363e01ea783062539d 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -311,11 +311,36 @@ __attribute__((weak)) void f(int x) { printf("%d", x); }
 AC_MSG_RESULT($ac_cv_c_weak_attribute)
 if test $ac_cv_c_weak_attribute = yes; then
   AC_DEFINE(HAVE_ATTR_WEAK, 1, [Whether the C compiler accepts the "weak" attribute])
+  AC_DEFINE(ATTR_WEAK, [__attribute__((weak))], [apply the weak attribute to a symbol])
 fi
 ])dnl End of CHECK_WEAK_ATTRIBUTE
 
 CHECK_WEAK_ATTRIBUTE
 
+AC_DEFUN([CHECK_NORETURN_ATTRIBUTE],
+[AC_REQUIRE([AC_PROG_CC])
+AC_MSG_CHECKING(whether the C compiler (${CC-cc}) accepts the "noreturn" attribute)
+AC_CACHE_VAL(ac_cv_c_noreturn_attribute,
+[ac_cv_c_noreturn_attribute=no
+AC_TRY_COMPILE(
+[ #include <stdio.h>
+__attribute__((noreturn)) void f(int x) { printf("%d", x); }
+], [
+   f(1);
+],
+[ac_cv_c_noreturn_attribute="yes"],
+[ac_cv_c_noreturn_attribute="no"])
+])
+
+AC_MSG_RESULT($ac_cv_c_noreturn_attribute)
+if test $ac_cv_c_noreturn_attribute = yes; then
+  AC_DEFINE(HAVE_ATTR_NORETURN, 1, [Whether the C compiler accepts the "noreturn" attribute])
+  AC_DEFINE(ATTR_NORETURN, [__attribute__((__noreturn__))], [apply the noreturn attribute to a function that exits the program])
+fi
+])dnl End of CHECK_NORETURN_ATTRIBUTE
+
+CHECK_NORETURN_ATTRIBUTE
+
 if test "$srcdir" != "."; then
        CPPFLAGS="$CPPFLAGS -I$srcdir"
 fi

diff --git a/daemon/unbound.c b/daemon/unbound.c
index 3f0f75a560c51ace918ce2d61aca03fb56cb91a7..020e453034ad59a79438634a39eea6e818a08e0d 100644 (file)
--- a/daemon/unbound.c
+++ b/daemon/unbound.c
@@ -730,7 +730,7 @@ main(int argc, char* argv[])
                }
        }
        argc -= optind;
-       argv += optind;
+       /* argv += optind; not using further arguments */
 
        if(winopt) {
 #ifdef UB_ON_WINDOWS

diff --git a/doc/Changelog b/doc/Changelog
index 38efef4ad9f56657859d60d1e677b15c997726f4..7cbc6ab265ec3cd4e2041ee98f68a74f8133cf9c 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,5 +1,9 @@
 13 September 2018: Wouter
        - Fix seed for random backup code to use explicit zero when wiped.
+       - exit log routine is annotated as noreturn function.
+       - free memory leaks in config strlist and str2list insert functions.
+       - do not move unused argv variable after getopt.
+       - Remove unused if clause in testcode.
 
 11 September 2018: Wouter
        - Fixed unused return value warnings in contrib/fastrpz.patch for

diff --git a/libunbound/libunbound.c b/libunbound/libunbound.c
index 275e8d25a1686bc31b5a61b0fdec053ea1f5297e..31adbd54dcdbc62841f8d8d03ee4aedf578d7073 100644 (file)
--- a/libunbound/libunbound.c
+++ b/libunbound/libunbound.c
@@ -109,13 +109,13 @@ static struct ub_ctx* ub_ctx_create_nopipe(void)
        alloc_init(&ctx->superalloc, NULL, 0);
        seed = (unsigned int)time(NULL) ^ (unsigned int)getpid();
        if(!(ctx->seed_rnd = ub_initstate(seed, NULL))) {
-               seed = 0;
+               explicit_bzero(&seed, sizeof(seed));
                ub_randfree(ctx->seed_rnd);
                free(ctx);
                errno = ENOMEM;
                return NULL;
        }
-       seed = 0;
+       explicit_bzero(&seed, sizeof(seed));
        lock_basic_init(&ctx->qqpipe_lock);
        lock_basic_init(&ctx->rrpipe_lock);
        lock_basic_init(&ctx->cfglock);
@@ -392,7 +392,6 @@ ub_ctx_add_ta(struct ub_ctx* ctx, const char* ta)
        }
        if(!cfg_strlist_insert(&ctx->env->cfg->trust_anchor_list, dup)) {
                lock_basic_unlock(&ctx->cfglock);
-               free(dup);
                return UB_NOMEM;
        }
        lock_basic_unlock(&ctx->cfglock);
@@ -412,7 +411,6 @@ ub_ctx_add_ta_file(struct ub_ctx* ctx, const char* fname)
        }
        if(!cfg_strlist_insert(&ctx->env->cfg->trust_anchor_file_list, dup)) {
                lock_basic_unlock(&ctx->cfglock);
-               free(dup);
                return UB_NOMEM;
        }
        lock_basic_unlock(&ctx->cfglock);
@@ -432,7 +430,6 @@ int ub_ctx_add_ta_autr(struct ub_ctx* ctx, const char* fname)
        if(!cfg_strlist_insert(&ctx->env->cfg->auto_trust_anchor_file_list,
                dup)) {
                lock_basic_unlock(&ctx->cfglock);
-               free(dup);
                return UB_NOMEM;
        }
        lock_basic_unlock(&ctx->cfglock);
@@ -452,7 +449,6 @@ ub_ctx_trustedkeys(struct ub_ctx* ctx, const char* fname)
        }
        if(!cfg_strlist_insert(&ctx->env->cfg->trusted_keys_file_list, dup)) {
                lock_basic_unlock(&ctx->cfglock);
-               free(dup);
                return UB_NOMEM;
        }
        lock_basic_unlock(&ctx->cfglock);
@@ -962,7 +958,6 @@ ub_ctx_set_fwd(struct ub_ctx* ctx, const char* addr)
                return UB_NOMEM;
        }
        if(!cfg_strlist_insert(&s->addrs, dupl)) {
-               free(dupl);
                lock_basic_unlock(&ctx->cfglock);
                errno=ENOMEM;
                return UB_NOMEM;
@@ -1045,7 +1040,6 @@ int ub_ctx_set_stub(struct ub_ctx* ctx, const char* zone, const char* addr,
        }
        if(!cfg_strlist_insert(&elem->addrs, a)) {
                lock_basic_unlock(&ctx->cfglock);
-               free(a);
                errno = ENOMEM;
                return UB_NOMEM;
        }
@@ -1233,7 +1227,6 @@ ub_ctx_hosts(struct ub_ctx* ctx, const char* fname)
                                ins)) {
                                lock_basic_unlock(&ctx->cfglock);
                                fclose(in);
-                               free(ins);
                                errno=ENOMEM;
                                return UB_NOMEM;
                        }

diff --git a/libunbound/libworker.c b/libunbound/libworker.c
index 05006a0ec44c0cbf897215baab1e70972e0a5ca0..065f0a7b141aab943b7ac55ce20487cab5907cfc 100644 (file)
--- a/libunbound/libworker.c
+++ b/libunbound/libworker.c
@@ -187,7 +187,7 @@ libworker_setup(struct ub_ctx* ctx, int is_bg, struct ub_event_base* eb)
                if(!w->is_bg || w->is_bg_thread) {
                        lock_basic_unlock(&ctx->cfglock);
                }
-               seed = 0;
+               explicit_bzero(&seed, sizeof(seed));
                libworker_delete(w);
                return NULL;
        }
@@ -207,7 +207,7 @@ libworker_setup(struct ub_ctx* ctx, int is_bg, struct ub_event_base* eb)
                        hash_set_raninit((uint32_t)ub_random(w->env->rnd));
                }
        }
-       seed = 0;
+       explicit_bzero(&seed, sizeof(seed));
 
        if(eb)
                w->base = comm_base_create_event(eb);

diff --git a/smallapp/unbound-anchor.c b/smallapp/unbound-anchor.c
index f398509018d92af3d960098ffaabf8b99a8742e0..cd2da149dbba8ecaf02d89af9dc111438f71cd30 100644 (file)
--- a/smallapp/unbound-anchor.c
+++ b/smallapp/unbound-anchor.c
@@ -2349,7 +2349,7 @@ int main(int argc, char* argv[])
                }
        }
        argc -= optind;
-       argv += optind;
+       /* argv += optind; not using further arguments */
        if(argc != 0)
                usage();
 

diff --git a/testcode/delayer.c b/testcode/delayer.c
index 5489b591e3377b87446dc89ac08dbc30cdd35adc..4abcfc235dcdcf569e76249393b9f0a9e6d9fb45 100644 (file)
--- a/testcode/delayer.c
+++ b/testcode/delayer.c
@@ -788,7 +788,7 @@ service_tcp_relay(struct tcp_proxy** tcp_proxies, struct timeval* now,
                        if(!tcp_relay_write(p->server_s, &p->querylist, 
                                &p->querylast, now))
                                delete_it = 1;
-                       if(p->querylist && p->server_s != -1 &&
+                       if(p->querylist &&
                                dl_tv_smaller(&p->querylist->wait, now))
                                FD_SET(FD_SET_T p->server_s, worig);
                        else    FD_CLR(FD_SET_T p->server_s, worig);

diff --git a/testcode/petal.c b/testcode/petal.c
index 1c26fa700346dd59dcd49736dbc2c57ca993edd1..e1f5f43417e3a94398742c1179cb801a01054cbc 100644 (file)
--- a/testcode/petal.c
+++ b/testcode/petal.c
@@ -417,7 +417,7 @@ provide_file_10(SSL* ssl, char* fname)
        }
        fclose(in);
        at += len;
-       avail -= len;
+       /* avail -= len; unused */
        if(SSL_write(ssl, buf, at-buf) <= 0) {
                /* write failure */
        }
@@ -506,7 +506,7 @@ provide_file_chunked(SSL* ssl, char* fname)
                        snprintf(at, avail, "\r\n");
                        r = strlen(at);
                        at += r;
-                       avail -= r;
+                       /* avail -= r; unused */
                }
                /* send chunk */
                if(SSL_write(ssl, buf, at-buf) <= 0) {
@@ -569,7 +569,9 @@ do_service(char* addr, int port, char* key, char* cert)
        while(go) {
                struct sockaddr_storage from;
                socklen_t flen = (socklen_t)sizeof(from);
-               int s = accept(fd, (struct sockaddr*)&from, &flen);
+               int s;
+               memset(&from, 0, sizeof(from));
+               s = accept(fd, (struct sockaddr*)&from, &flen);
                if(verb) fflush(stdout);
                if(s != -1) {
                        SSL* ssl = setup_ssl(s, sslctx);
@@ -633,7 +635,7 @@ int main(int argc, char* argv[])
                }
        }
        argc -= optind;
-       argv += optind;
+       /* argv += optind; not using further arguments */
        if(argc != 0)
                usage();
 

diff --git a/testcode/testbound.c b/testcode/testbound.c
index 071ac9c2a5b44b389a3239d425e88b0b321ad3bf..cea74c593540cd7b90260f30f156a015baa93991 100644 (file)
--- a/testcode/testbound.c
+++ b/testcode/testbound.c
@@ -429,14 +429,14 @@ main(int argc, char* argv[])
                case 'h':
                default:
                        testbound_usage();
-                       return 1;
+                       exit(1);
                }
        }
        argc -= optind;
-       argv += optind;
+       /* argv += optind; not using further arguments */
        if(argc != 0) {
                testbound_usage();
-               return 1;
+               exit(1);
        }
        log_info("Start of %s testbound program.", PACKAGE_STRING);
        if(atexit(&remove_configfile) != 0)

diff --git a/testcode/testpkts.c b/testcode/testpkts.c
index ec0f7fe2449a5b7deff0a5836f53dd7f14f3e14b..01f23e48ed2e73216e920f030c143afee1667383 100644 (file)
--- a/testcode/testpkts.c
+++ b/testcode/testpkts.c
@@ -46,6 +46,7 @@ enum verbosity_value { NO_VERBOSE=0 };
 #endif
 /** logging routine, provided by caller */
 void verbose(enum verbosity_value lvl, const char* msg, ...) ATTR_FORMAT(printf, 2, 3);
+static void error(const char* msg, ...) ATTR_NORETURN;
 
 /** print error and exit */
 static void error(const char* msg, ...)

diff --git a/util/config_file.c b/util/config_file.c
index 5dffa37ebb5a98c67d6c443f5f71807e6e35e1f4..e6fbaeef40f3deefc13f2f6bff074cc494c8fe87 100644 (file)
--- a/util/config_file.c
+++ b/util/config_file.c
@@ -1578,11 +1578,15 @@ int
 cfg_strlist_insert(struct config_strlist** head, char* item)
 {
        struct config_strlist *s;
-       if(!item || !head)
+       if(!item || !head) {
+               free(item);
                return 0;
+       }
        s = (struct config_strlist*)calloc(1, sizeof(struct config_strlist));
-       if(!s)
+       if(!s) {
+               free(item);
                return 0;
+       }
        s->str = item;
        s->next = *head;
        *head = s;
@@ -1593,11 +1597,17 @@ int
 cfg_str2list_insert(struct config_str2list** head, char* item, char* i2)
 {
        struct config_str2list *s;
-       if(!item || !i2 || !head)
+       if(!item || !i2 || !head) {
+               free(item);
+               free(i2);
                return 0;
+       }
        s = (struct config_str2list*)calloc(1, sizeof(struct config_str2list));
-       if(!s)
+       if(!s) {
+               free(item);
+               free(i2);
                return 0;
+       }
        s->str = item;
        s->str2 = i2;
        s->next = *head;

diff --git a/util/config_file.h b/util/config_file.h
index d1bce1b69be49017ca8c69017fbb3d7f1a03fcaf..54edec410c3481383b114fa35abb91e986d35eeb 100644 (file)
--- a/util/config_file.h
+++ b/util/config_file.h
@@ -809,6 +809,7 @@ struct config_strlist* cfg_strlist_find(struct config_strlist* head,
  * @param head: pointer to strlist head variable.
  * @param item: new item. malloced by caller. If NULL the insertion fails.
  * @return: true on success.
+ * on fail, the item is free()d.
  */
 int cfg_strlist_insert(struct config_strlist** head, char* item);
 
@@ -822,6 +823,7 @@ int cfg_region_strlist_insert(struct regional* region,
  * @param item: new item. malloced by caller. If NULL the insertion fails.
  * @param i2: 2nd string, malloced by caller. If NULL the insertion fails.
  * @return: true on success.
+ * on fail, the item and i2 are free()d.
  */
 int cfg_str2list_insert(struct config_str2list** head, char* item, char* i2);
 

diff --git a/util/log.h b/util/log.h
index 7bc3d9e76152dc66410766123e3cb6532ad67271..f73c0754d657f0661d0afc3922db9f82c01e3277 100644 (file)
--- a/util/log.h
+++ b/util/log.h
@@ -174,7 +174,7 @@ void log_buf(enum verbosity_value level, const char* msg, struct sldns_buffer* b
  * Pass printf formatted arguments. No trailing newline is needed.
  * @param format: printf-style format string. Arguments follow.
  */
-void fatal_exit(const char* format, ...) ATTR_FORMAT(printf, 1, 2);
+void fatal_exit(const char* format, ...) ATTR_FORMAT(printf, 1, 2) ATTR_NORETURN;
 
 /**
  * va_list argument version of log_info.