BUILD: tree-wide: avoid warnings caused by redundant checks of obj_types

At many places we use construct such as:

   if (objt_server(blah))
       do_something(objt_server(blah));

At -O2 the compiler manages to simplify the operation and see that the
second one returns the same result as the first one. But at -O1 that's
not always the case, and the compiler is able to emit a second
expression and sees the potential null that results from it, and may
warn about a potential null deref (e.g. with gcc-6.5). There are two
solutions to this:
  - either the result of the first test has to be passed to a local
    variable
  - or the second reference ought to be unchecked using the __objt_*
    variant.

This patch fixes all occurrences at once by taking the second approach
(the least intrusive). For constructs like:

   objt_server(blah) ? objt_server(blah)->name : "no name"

a macro could be useful. It would for example take the object type
(server), the field name (name) and the default value. But there
are probably not enough occurrences across the whole code for this
to really matter.

This should be backported wherever it applies.

diff --git a/src/backend.c b/src/backend.c
index 2d2c0d6e093d4373de623fa80ceca2e1d236b596..fb6313181f1410b8157133ec0e32c7ba6fbc74e3 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -2230,7 +2230,7 @@ void back_handle_st_cer(struct stream *s)
 
        /* we probably have to release last stream from the server */
        if (objt_server(s->target)) {
-               health_adjust(objt_server(s->target), HANA_STATUS_L4_ERR);
+               health_adjust(__objt_server(s->target), HANA_STATUS_L4_ERR);
 
                if (s->flags & SF_CURR_SESS) {
                        s->flags &= ~SF_CURR_SESS;

diff --git a/src/http_ana.c b/src/http_ana.c
index 036ae109d141c93747967d7f9ec025aec47031b4..9c3e68a1be2a4cf9a838ea22b71ea85405f30388 100644 (file)
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -1328,7 +1328,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
                        struct connection *conn = NULL;
 
                        if (objt_cs(s->si[1].end))
-                               conn = objt_cs(s->si[1].end)->conn;
+                               conn = __objt_cs(s->si[1].end)->conn;
 
                        /* Perform a L7 retry because server refuses the early data. */
                        if ((si_b->flags & SI_FL_L7_RETRY) &&
@@ -1889,13 +1889,13 @@ int http_process_res_common(struct stream *s, struct channel *rep, int an_bit, s
                 * requests and this one isn't. Note that servers which don't have cookies
                 * (eg: some backup servers) will return a full cookie removal request.
                 */
-               if (!objt_server(s->target)->cookie) {
+               if (!__objt_server(s->target)->cookie) {
                        chunk_printf(&trash,
                                     "%s=; Expires=Thu, 01-Jan-1970 00:00:01 GMT; path=/",
                                     s->be->cookie_name);
                }
                else {
-                       chunk_printf(&trash, "%s=%s", s->be->cookie_name, objt_server(s->target)->cookie);
+                       chunk_printf(&trash, "%s=%s", s->be->cookie_name, __objt_server(s->target)->cookie);
 
                        if (s->be->cookie_maxidle || s->be->cookie_maxlife) {
                                /* emit last_date, which is mandatory */
@@ -1968,10 +1968,10 @@ int http_process_res_common(struct stream *s, struct channel *rep, int an_bit, s
                 * the 'checkcache' option, and send an alert.
                 */
                ha_alert("Blocking cacheable cookie in response from instance %s, server %s.\n",
-                        s->be->id, objt_server(s->target) ? objt_server(s->target)->id : "<dispatch>");
+                        s->be->id, objt_server(s->target) ? __objt_server(s->target)->id : "<dispatch>");
                send_log(s->be, LOG_ALERT,
                         "Blocking cacheable cookie in response from instance %s, server %s.\n",
-                        s->be->id, objt_server(s->target) ? objt_server(s->target)->id : "<dispatch>");
+                        s->be->id, objt_server(s->target) ? __objt_server(s->target)->id : "<dispatch>");
                goto deny;
        }
 
@@ -5006,8 +5006,8 @@ static void http_debug_stline(const char *dir, struct stream *s, const struct ht
 
         chunk_printf(&trash, "%08x:%s.%s[%04x:%04x]: ", s->uniq_id, s->be->id,
                      dir,
-                     objt_conn(sess->origin) ? (unsigned short)objt_conn(sess->origin)->handle.fd : -1,
-                     objt_cs(s->si[1].end) ? (unsigned short)objt_cs(s->si[1].end)->conn->handle.fd : -1);
+                     objt_conn(sess->origin) ? (unsigned short)__objt_conn(sess->origin)->handle.fd : -1,
+                     objt_cs(s->si[1].end) ? (unsigned short)__objt_cs(s->si[1].end)->conn->handle.fd : -1);
 
         max = HTX_SL_P1_LEN(sl);
         UBOUND(max, trash.size - trash.data - 3);
@@ -5037,8 +5037,8 @@ static void http_debug_hdr(const char *dir, struct stream *s, const struct ist n
 
         chunk_printf(&trash, "%08x:%s.%s[%04x:%04x]: ", s->uniq_id, s->be->id,
                      dir,
-                     objt_conn(sess->origin) ? (unsigned short)objt_conn(sess->origin)->handle.fd : -1,
-                     objt_cs(s->si[1].end) ? (unsigned short)objt_cs(s->si[1].end)->conn->handle.fd : -1);
+                     objt_conn(sess->origin) ? (unsigned short)__objt_conn(sess->origin)->handle.fd : -1,
+                     objt_cs(s->si[1].end) ? (unsigned short)__objt_cs(s->si[1].end)->conn->handle.fd : -1);
 
         max = n.len;
         UBOUND(max, trash.size - trash.data - 3);

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 83d93fd64a8434525115a5a2402480c480a5f4aa..4aaac9d82f24baa4044c15c54b2db0fe086a0bfc 100644 (file)
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -271,11 +271,11 @@ int tcp_connect_server(struct connection *conn, int flags)
 
        switch (obj_type(conn->target)) {
        case OBJ_TYPE_PROXY:
-               be = objt_proxy(conn->target);
+               be = __objt_proxy(conn->target);
                srv = NULL;
                break;
        case OBJ_TYPE_SERVER:
-               srv = objt_server(conn->target);
+               srv = __objt_server(conn->target);
                be = srv->proxy;
                /* Make sure we check that we have data before activating
                 * TFO, or we could trigger a kernel issue whereby after

diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index b615a8eae56063b99bcc2b3f8e7ca2393b6a2a0e..663369db435341dc60ede186405d39b1804893ea 100644 (file)
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -214,11 +214,11 @@ static int uxst_connect_server(struct connection *conn, int flags)
 
        switch (obj_type(conn->target)) {
        case OBJ_TYPE_PROXY:
-               be = objt_proxy(conn->target);
+               be = __objt_proxy(conn->target);
                srv = NULL;
                break;
        case OBJ_TYPE_SERVER:
-               srv = objt_server(conn->target);
+               srv = __objt_server(conn->target);
                be = srv->proxy;
                break;
        default:

diff --git a/src/stream.c b/src/stream.c
index 04d2845651f1dc1b3bee76a134cf4d27288db740..8f1da7625704fc0185e74d2752e2ffb92b07ed9c 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -624,8 +624,8 @@ static void stream_free(struct stream *s)
                        s->flags &= ~SF_CURR_SESS;
                        _HA_ATOMIC_DEC(&__objt_server(s->target)->cur_sess);
                }
-               if (may_dequeue_tasks(objt_server(s->target), s->be))
-                       process_srv_queue(objt_server(s->target));
+               if (may_dequeue_tasks(__objt_server(s->target), s->be))
+                       process_srv_queue(__objt_server(s->target));
        }
 
        if (unlikely(s->srv_conn)) {
@@ -826,7 +826,7 @@ void stream_process_counters(struct stream *s)
                _HA_ATOMIC_ADD(&s->be->be_counters.bytes_in,    bytes);
 
                if (objt_server(s->target))
-                       _HA_ATOMIC_ADD(&objt_server(s->target)->counters.bytes_in, bytes);
+                       _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.bytes_in, bytes);
 
                if (sess->listener && sess->listener->counters)
                        _HA_ATOMIC_ADD(&sess->listener->counters->bytes_in, bytes);
@@ -844,7 +844,7 @@ void stream_process_counters(struct stream *s)
                _HA_ATOMIC_ADD(&s->be->be_counters.bytes_out,    bytes);
 
                if (objt_server(s->target))
-                       _HA_ATOMIC_ADD(&objt_server(s->target)->counters.bytes_out, bytes);
+                       _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.bytes_out, bytes);
 
                if (sess->listener && sess->listener->counters)
                        _HA_ATOMIC_ADD(&sess->listener->counters->bytes_out, bytes);
@@ -916,7 +916,7 @@ static void back_establish(struct stream *s)
        }
 
        if (objt_server(s->target))
-               health_adjust(objt_server(s->target), HANA_STATUS_L4_OK);
+               health_adjust(__objt_server(s->target), HANA_STATUS_L4_OK);
 
        if (!IS_HTX_STRM(s)) { /* let's allow immediate data connection in this case */
                /* if the user wants to log as soon as possible, without counting
@@ -1439,7 +1439,7 @@ static int process_store_rules(struct stream *s, struct channel *rep, int an_bit
                struct dict_entry *de;
                struct stktable *t = s->store[i].table;
 
-               if (objt_server(s->target) && objt_server(s->target)->flags & SRV_F_NON_STICK) {
+               if (objt_server(s->target) && __objt_server(s->target)->flags & SRV_F_NON_STICK) {
                        stksess_free(s->store[i].table, s->store[i].ts);
                        s->store[i].ts = NULL;
                        continue;
@@ -2428,8 +2428,8 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
                    si_b->prev_state == SI_ST_EST) {
                        chunk_printf(&trash, "%08x:%s.srvcls[%04x:%04x]\n",
                                      s->uniq_id, s->be->id,
-                                     objt_cs(si_f->end) ? (unsigned short)objt_cs(si_f->end)->conn->handle.fd : -1,
-                                     objt_cs(si_b->end) ? (unsigned short)objt_cs(si_b->end)->conn->handle.fd : -1);
+                                     objt_cs(si_f->end) ? (unsigned short)__objt_cs(si_f->end)->conn->handle.fd : -1,
+                                     objt_cs(si_b->end) ? (unsigned short)__objt_cs(si_b->end)->conn->handle.fd : -1);
                        DISGUISE(write(1, trash.area, trash.data));
                }
 
@@ -2437,8 +2437,8 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
                    si_f->prev_state == SI_ST_EST) {
                        chunk_printf(&trash, "%08x:%s.clicls[%04x:%04x]\n",
                                      s->uniq_id, s->be->id,
-                                     objt_cs(si_f->end) ? (unsigned short)objt_cs(si_f->end)->conn->handle.fd : -1,
-                                     objt_cs(si_b->end) ? (unsigned short)objt_cs(si_b->end)->conn->handle.fd : -1);
+                                     objt_cs(si_f->end) ? (unsigned short)__objt_cs(si_f->end)->conn->handle.fd : -1,
+                                     objt_cs(si_b->end) ? (unsigned short)__objt_cs(si_b->end)->conn->handle.fd : -1);
                        DISGUISE(write(1, trash.area, trash.data));
                }
        }
@@ -2505,8 +2505,8 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
                     (!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE)))) {
                chunk_printf(&trash, "%08x:%s.closed[%04x:%04x]\n",
                              s->uniq_id, s->be->id,
-                             objt_cs(si_f->end) ? (unsigned short)objt_cs(si_f->end)->conn->handle.fd : -1,
-                             objt_cs(si_b->end) ? (unsigned short)objt_cs(si_b->end)->conn->handle.fd : -1);
+                             objt_cs(si_f->end) ? (unsigned short)__objt_cs(si_f->end)->conn->handle.fd : -1,
+                             objt_cs(si_b->end) ? (unsigned short)__objt_cs(si_b->end)->conn->handle.fd : -1);
                DISGUISE(write(1, trash.area, trash.data));
        }
 
@@ -3213,8 +3213,8 @@ static int stats_dump_full_strm_to_buffer(struct stream_interface *si, struct st
                if (strm->be->cap & PR_CAP_BE)
                        chunk_appendf(&trash,
                                     "  server=%s (id=%u)",
-                                    objt_server(strm->target) ? objt_server(strm->target)->id : "<none>",
-                                    objt_server(strm->target) ? objt_server(strm->target)->puid : 0);
+                                    objt_server(strm->target) ? __objt_server(strm->target)->id : "<none>",
+                                    objt_server(strm->target) ? __objt_server(strm->target)->puid : 0);
                else
                        chunk_appendf(&trash, "  server=<NONE> (id=-1)");
 
@@ -3590,7 +3590,7 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
                                             get_host_port(conn->src),
                                             strm_fe(curr_strm)->id,
                                             (curr_strm->be->cap & PR_CAP_BE) ? curr_strm->be->id : "<NONE>",
-                                            objt_server(curr_strm->target) ? objt_server(curr_strm->target)->id : "<none>"
+                                            objt_server(curr_strm->target) ? __objt_server(curr_strm->target)->id : "<none>"
                                             );
                                break;
                        case AF_UNIX:
@@ -3599,7 +3599,7 @@ static int cli_io_handler_dump_sess(struct appctx *appctx)
                                             strm_li(curr_strm)->luid,
                                             strm_fe(curr_strm)->id,
                                             (curr_strm->be->cap & PR_CAP_BE) ? curr_strm->be->id : "<NONE>",
-                                            objt_server(curr_strm->target) ? objt_server(curr_strm->target)->id : "<none>"
+                                            objt_server(curr_strm->target) ? __objt_server(curr_strm->target)->id : "<none>"
                                             );
                                break;
                        }

diff --git a/src/stream_interface.c b/src/stream_interface.c
index 8ad1854970f6fb86323720b8cbcf828ab79dc043..b78c3bc48e5117fbb6c8a19eff9f6bc304d15309 100644 (file)
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -434,7 +434,7 @@ static void stream_int_notify(struct stream_interface *si)
 
        /* process consumer side */
        if (channel_is_empty(oc)) {
-               struct connection *conn = objt_cs(si->end) ? objt_cs(si->end)->conn : NULL;
+               struct connection *conn = objt_cs(si->end) ? __objt_cs(si->end)->conn : NULL;
 
                if (((oc->flags & (CF_SHUTW|CF_SHUTW_NOW)) == CF_SHUTW_NOW) &&
                    (si->state == SI_ST_EST) && (!conn || !(conn->flags & (CO_FL_WAIT_XPRT | CO_FL_EARLY_SSL_HS))))