Elukey has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/314662

Change subject: Add extra compiler warnings to the Makefile
......................................................................

Add extra compiler warnings to the Makefile

This change adds -Wextra to the Makefile's CFLAGS in order to include
more and stricter checks at compile time. All the changes in the diff
are related to warnings issued by the compiler.
Re-added the -g compiler option (it was removed a while ago) because
it shouldn't give any big overhead to the final binary, but it allows
a better debugging experience especially in live production systems.

Bug: T147436
Change-Id: I45e39c29941981a552175664af4db562746cd0d0
---
M Makefile
M varnishkafka.c
M varnishkafka.h
3 files changed, 43 insertions(+), 41 deletions(-)


  git pull 
ssh://gerrit.wikimedia.org:29418/operations/software/varnish/varnishkafka 
refs/changes/62/314662/1

diff --git a/Makefile b/Makefile
index aa344b8..8e6511b 100644
--- a/Makefile
+++ b/Makefile
@@ -18,7 +18,7 @@
 CFLAGS  += -DVARNISHKAFKA_CONF_PATH=\"$(CFPATH)\"
 CFLAGS  += -I/usr/include/varnish/
 
-CFLAGS  += -Wall -Werror -O2
+CFLAGS  += -Wall -Wextra -Werror -O2 -g
 LDFLAGS += -lyajl
 LDFLAGS += -lrdkafka -lvarnishapi -lpthread -lrt -lz -lm
 
diff --git a/varnishkafka.c b/varnishkafka.c
index f37bdab..152d735 100644
--- a/varnishkafka.c
+++ b/varnishkafka.c
@@ -257,7 +257,7 @@
        tag->flags  = tag_flags;
 
        if (var) {
-               if (varlen == -1)
+               if (!varlen)
                        varlen = strlen(var);
 
                tag->var = malloc(varlen+1);
@@ -553,7 +553,7 @@
        struct tm tm;
        const char *timefmt = "[%d/%b/%Y:%T %z]";
        const int timelen   = 64;
-       size_t tlen;
+       size_t tlen = len; // unused parameter, prevents compiler warnings
 
        /*
         * The special format for tag->var "end:strftime" is used
@@ -819,95 +819,97 @@
                 */
                ['b'] = { {
                                /* Size of response in bytes, with HTTP 
headers. */
-                               { VSL_CLIENTMARKER, SLT_ReqAcct, .col = 5}
-                       } },
+                               { .spec = VSL_CLIENTMARKER, .tag = SLT_ReqAcct, 
.col = 5}
+                       } , .def = NULL },
                ['D'] = { {
                                /* Time taken to serve the request (s) */
-                               { VSL_CLIENTMARKER, SLT_Timestamp,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_Timestamp,
                                  .var = "Resp", .col = 2,
                                  .parser = parse_DT}
-                       } },
+                       } , .def = NULL },
                ['T'] = { {
                                /* Time taken to serve the request (s) */
-                               { VSL_CLIENTMARKER, SLT_Timestamp,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_Timestamp,
                                  .var = "Resp", .col = 2,
                                  .parser = parse_DT}
-                       } },
+                       } , .def = NULL },
                ['H'] = { {
                                /* The request protocol */
-                               { VSL_CLIENTMARKER, SLT_ReqProtocol },
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_ReqProtocol },
                        }, .def = "HTTP/1.0" },
                ['h'] = { {
                                /* Remote hostname (IP address) */
-                               { VSL_CLIENTMARKER, SLT_ReqStart, .col = 1},
-                       } },
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_ReqStart, .col = 1},
+                       } , .def = NULL },
                ['i'] = { {
                                /* Used as %{VARNAME}i.
                                 * The contents of VARNAME: header line(s)
                                 * in the request sent to the server.
                                 */
-                               { VSL_CLIENTMARKER, SLT_ReqHeader,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_ReqHeader,
                                  .tag_flags = TAG_F_LAST }
-                       } },
+                       } , .def = NULL },
                ['l'] = { {
-                               { VSL_CLIENTMARKER }
+                               { .spec = VSL_CLIENTMARKER }
                        }, .def = conf.logname },
                ['m'] = { {
                                /* Request method (GET|POST|..) */
-                               { VSL_CLIENTMARKER, SLT_ReqMethod }
-                       } },
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_ReqMethod }
+                       } , .def = NULL },
                ['q'] = { {
                                /* The request query string */
-                               { VSL_CLIENTMARKER, SLT_ReqURL, .parser = 
parse_q }
+                               { .spec = VSL_CLIENTMARKER, .tag = SLT_ReqURL,
+                                 .parser = parse_q }
                        },  .def = "" },
                ['o'] = { {
                                /* Used as %{VARNAME}o.
                                 * The contents of VARNAME: header line(s)
                                 * in the response sent to the server.
                                 */
-                               { VSL_CLIENTMARKER, SLT_RespHeader,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_RespHeader,
                                  .tag_flags = TAG_F_LAST }
-                       } },
+                       } , .def = NULL },
                ['s'] = { {
                                /* The response HTTP status */
-                               { VSL_CLIENTMARKER, SLT_RespStatus,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_RespStatus,
                                  .tag_flags = TAG_F_LAST }
-                       } },
+                       } , .def = NULL },
                ['t'] = { {
-                               { VSL_CLIENTMARKER, SLT_Timestamp,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_Timestamp,
                                  .col = 2,
                                  .parser = parse_t,
                                  .tag_flags = TAG_F_TIMESTAMP }
-                       } },
+                       } , .def = NULL },
                ['U'] = { {
                                /* The URL path requested, not including any 
query string. */
-                               { VSL_CLIENTMARKER, SLT_ReqURL, .parser = 
parse_U }
-                       } },
+                               { .spec = VSL_CLIENTMARKER, .tag = SLT_ReqURL,
+                                 .parser = parse_U }
+                       } , .def = NULL },
                ['u'] = { {
-                               { VSL_CLIENTMARKER, SLT_RespHeader,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_RespHeader,
                                  .var = "authorization",
                                  .parser = parse_auth_user }
-                       } },
+                       } , .def = NULL },
                ['x'] = { {
                                /* Various Varnish related tags */
-                               { VSL_CLIENTMARKER, SLT_Timestamp,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
SLT_Timestamp,
                                  .var = "Process",
                                  .fmtvar = "Varnish:time_firstbyte", .col = 2 
},
-                               { VSL_CLIENTMARKER, SLT_Begin,
+                               { .spec = VSL_CLIENTMARKER, .tag = SLT_Begin,
                                  .fmtvar = "Varnish:xvid", .col = 2 },
-                               { VSL_CLIENTMARKER, SLT_VCL_call,
+                               { .spec = VSL_CLIENTMARKER, .tag = SLT_VCL_call,
                                  .fmtvar = "Varnish:handling",
                                  .parser = parse_vcl_handling },
-                               { VSL_CLIENTMARKER, SLT_VCL_Log,
+                               { .spec = VSL_CLIENTMARKER, .tag = SLT_VCL_Log,
                                  .fmtvar = "VCL_Log:*" },
-                               { VSL_CLIENTMARKER, SLT_VSL,
+                               { .spec = VSL_CLIENTMARKER, .tag = SLT_VSL,
                                  .fmtvar = "VSL_API:*",
                                  .tag_flags = TAG_F_MATCH_PREFIX },
-                       } },
+                       } , .def = NULL },
                ['n'] = { {
-                               { VSL_CLIENTMARKER, VSL_TAG__ONCE,
+                               { .spec = VSL_CLIENTMARKER, .tag = 
VSL_TAG__ONCE,
                                  .parser = parse_seq }
-                       } },
+                       } , .def = NULL },
        };
        /* Replace legacy formatters */
        static const char *replace[] = {
@@ -927,7 +929,7 @@
        while (*s) {
                const char *begin;
                const char *var = NULL;
-               int varlen = 0;
+               size_t varlen = 0;
                const char *def = NULL;
                int deflen = -1;
                const char *name = NULL;
@@ -1133,7 +1135,7 @@
                                if ((iswc = strstr(map[(int)*s].f[i].fmtvar,
                                                   ":*"))) {
                                        /* Wildcard definition */
-                                       int fvlen = (int)(iswc -
+                                       size_t fvlen = (int)(iswc -
                                                          map[(int)*s].
                                                          f[i].fmtvar) + 1;
 
@@ -1166,7 +1168,7 @@
                                    map[(int)*s].f[i].spec,
                                    map[(int)*s].f[i].tag,
                                    var ? var : map[(int)*s].f[i].var,
-                                   var ? varlen : -1,
+                                   var ? varlen : 0,
                                    map[(int)*s].f[i].col,
                                    map[(int)*s].f[i].parser,
                                    map[(int)*s].f[i].tag_flags
@@ -1535,7 +1537,7 @@
                         *    the current tag candidate for the match
                         *        must be 0 (so equal strings).
                         */
-                       if (tag->varlen != (int)(t-ptr) ||
+                       if (tag->varlen != (size_t)(t-ptr) ||
                            strncasecmp(ptr, tag->var, tag->varlen))
                                continue;
 
diff --git a/varnishkafka.h b/varnishkafka.h
index ef99b2c..681f875 100644
--- a/varnishkafka.h
+++ b/varnishkafka.h
@@ -190,7 +190,7 @@
        int         log_to;
 #define VK_LOG_STDERR 0x1
 #define VK_LOG_SYSLOG 0x2
-       int         log_rate;        /* Maximum log rate per minute. */
+       size_t      log_rate;        /* Maximum log rate per minute. */
        int         log_rate_period; /* Log rate limiting period */
 
        int         log_kafka_msg_error;  /* Log Kafka message delivery errors*/

-- 
To view, visit https://gerrit.wikimedia.org/r/314662
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I45e39c29941981a552175664af4db562746cd0d0
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/varnish/varnishkafka
Gerrit-Branch: master
Gerrit-Owner: Elukey <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to