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.

(cherry picked from commit 88bc800eae8d0a2118a273afc52ecdc529c9f523)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/backend.c b/src/backend.c
index 2d2c0d6..fb63131 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 036ae10..9c3e68a 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 83d93fd..4aaac9d 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 b615a8e..663369d 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 04d2845..8f1da76 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 8ad1854..b78c3bc 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))))