From 508c39ee133d6725a4440ba98a1555e2026975c2 Mon Sep 17 00:00:00 2001 From: Martin Maly Date: Wed, 6 May 2015 13:18:33 -0700 Subject: [PATCH] Resolve compilation errors if compiled with more stringent semantic checks. Adding Travis test to build with strict warnings. Fixing a warning in a test which used signed/unsigned integer comparison. --- .travis.yml | 1 + Makefile | 8 ++++++++ tests/json/test_json.cc | 2 +- travis.sh | 10 ++++++++++ upb/handlers.c | 2 +- upb/json/parser.c | 17 +++++++++++------ upb/json/parser.rl | 9 ++++++++- upb/json/printer.c | 2 +- upb/pb/decoder.c | 6 +++--- upb/pb/encoder.c | 4 ++-- upb/table.c | 2 +- 11 files changed, 47 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4407f5c6ad..917aa91e3e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,3 +10,4 @@ env: - UPB_TRAVIS_BUILD=withprotobuf - UPB_TRAVIS_BUILD=lua - UPB_TRAVIS_BUILD=coverage + - UPB_TRAVIS_BUILD=warnings diff --git a/Makefile b/Makefile index 8af52d4fb1..5887fbe8d5 100644 --- a/Makefile +++ b/Makefile @@ -38,6 +38,10 @@ USER_CPPFLAGS= # Build with "make WITH_JIT=yes" (or anything besides "no") to enable the JIT. WITH_JIT=no +# Build with "make WITH_MAX_WARNINGS=yes" (or anything besides "no") to enable +# with strict warnings and treat warnings as errors. +WITH_MAX_WARNINGS=no + # Basic compiler/flag setup. CC=cc CXX=c++ @@ -53,6 +57,10 @@ ifneq ($(WITH_JIT), no) CPPFLAGS += -DUPB_USE_JIT_X64 endif +ifneq ($(WITH_MAX_WARNINGS), no) + WARNFLAGS=-Wall -Wextra -Wpointer-arith -Werror +endif + # Build with "make Q=" to see all commands that are being executed. Q=@ diff --git a/tests/json/test_json.cc b/tests/json/test_json.cc index 4465c76056..828e603e29 100644 --- a/tests/json/test_json.cc +++ b/tests/json/test_json.cc @@ -347,7 +347,7 @@ void test_json_roundtrip() { test_case->input : test_case->expected; - for (int i = 0; i < strlen(test_case->input); i++) { + for (size_t i = 0; i < strlen(test_case->input); i++) { test_json_roundtrip_message(test_case->input, expected, serialize_handlers.get(), i); } diff --git a/travis.sh b/travis.sh index 26cf4817af..502b2f1640 100755 --- a/travis.sh +++ b/travis.sh @@ -28,6 +28,16 @@ withprotobuf_script() { make test } +# Build with strict warnings. +warnings_install() { + : +} +warnings_script() { + make -j12 default WITH_MAX_WARNINGS=yes + make -j12 tests WITH_MAX_WARNINGS=yes + make test +} + # A 32-bit build. Can only test the core because any dependencies # need to be available as 32-bit libs also, which gets hairy fast. # Can't enable the JIT because it only supports x64. diff --git a/upb/handlers.c b/upb/handlers.c index fd19633c33..fe368e57d0 100644 --- a/upb/handlers.c +++ b/upb/handlers.c @@ -574,7 +574,7 @@ bool upb_handlers_getselector(const upb_fielddef *f, upb_handlertype_t type, *s = f->selector_base; break; } - assert(*s < upb_fielddef_containingtype(f)->selector_count); + assert((size_t)*s < upb_fielddef_containingtype(f)->selector_count); return true; } diff --git a/upb/json/parser.c b/upb/json/parser.c index a1e83c5baf..08cd13da6e 100644 --- a/upb/json/parser.c +++ b/upb/json/parser.c @@ -1135,8 +1135,6 @@ static const char _json_trans_actions[] = { }; static const int json_start = 1; -static const int json_first_final = 56; -static const int json_error = 0; static const int json_en_number_machine = 10; static const int json_en_string_machine = 19; @@ -1164,7 +1162,7 @@ size_t parse(void *closure, const void *hd, const char *buf, size_t size, capture_resume(parser, buf); -#line 1168 "upb/json/parser.c" +#line 1166 "upb/json/parser.c" { int _klen; unsigned int _trans; @@ -1350,7 +1348,7 @@ _match: #line 1082 "upb/json/parser.rl" { p--; {cs = stack[--top]; goto _again;} } break; -#line 1354 "upb/json/parser.c" +#line 1352 "upb/json/parser.c" } } @@ -1382,6 +1380,13 @@ error: bool end(void *closure, const void *hd) { UPB_UNUSED(closure); UPB_UNUSED(hd); + + // Prevent compile warning on unused static constants. + UPB_UNUSED(json_start); + UPB_UNUSED(json_en_number_machine); + UPB_UNUSED(json_en_string_machine); + UPB_UNUSED(json_en_value_machine); + UPB_UNUSED(json_en_main); return true; } @@ -1414,13 +1419,13 @@ void upb_json_parser_reset(upb_json_parser *p) { int top; // Emit Ragel initialization of the parser. -#line 1418 "upb/json/parser.c" +#line 1423 "upb/json/parser.c" { cs = json_start; top = 0; } -#line 1157 "upb/json/parser.rl" +#line 1164 "upb/json/parser.rl" p->current_state = cs; p->parser_top = top; accumulate_clear(p); diff --git a/upb/json/parser.rl b/upb/json/parser.rl index 0379be450c..b171617818 100644 --- a/upb/json/parser.rl +++ b/upb/json/parser.rl @@ -1084,7 +1084,7 @@ static void end_object(upb_json_parser *p) { main := ws object ws; }%% -%% write data; +%% write data noerror nofinal; size_t parse(void *closure, const void *hd, const char *buf, size_t size, const upb_bufhandle *handle) { @@ -1122,6 +1122,13 @@ error: bool end(void *closure, const void *hd) { UPB_UNUSED(closure); UPB_UNUSED(hd); + + // Prevent compile warning on unused static constants. + UPB_UNUSED(json_start); + UPB_UNUSED(json_en_number_machine); + UPB_UNUSED(json_en_string_machine); + UPB_UNUSED(json_en_value_machine); + UPB_UNUSED(json_en_main); return true; } diff --git a/upb/json/printer.c b/upb/json/printer.c index deae058d3a..c7267e0c88 100644 --- a/upb/json/printer.c +++ b/upb/json/printer.c @@ -162,7 +162,7 @@ static bool putkey(void *closure, const void *handler_data) { return true; } -#define CHKFMT(val) if ((val) == -1) return false; +#define CHKFMT(val) if ((val) == (size_t)-1) return false; #define CHK(val) if (!(val)) return false; #define TYPE_HANDLERS(type, fmt_func) \ diff --git a/upb/pb/decoder.c b/upb/pb/decoder.c index 04ca41372b..522b02ea0f 100644 --- a/upb/pb/decoder.c +++ b/upb/pb/decoder.c @@ -124,7 +124,7 @@ static bool in_residual_buf(const upb_pbdecoder *d, const char *p) { // and the parsing stack, so must be called whenever either is updated. static void set_delim_end(upb_pbdecoder *d) { size_t delim_ofs = d->top->end_ofs - d->bufstart_ofs; - if (delim_ofs <= (d->end - d->buf)) { + if (delim_ofs <= (size_t)(d->end - d->buf)) { d->delim_end = d->buf + delim_ofs; d->data_end = d->delim_end; } else { @@ -268,7 +268,7 @@ static NOINLINE int32_t getbytes_slow(upb_pbdecoder *d, void *buf, advancetobuf(d, d->buf_param, d->size_param); } if (curbufleft(d) >= bytes) { - consumebytes(d, buf + avail, bytes); + consumebytes(d, (char *)buf + avail, bytes); return DECODE_OK; } else if (d->data_end == d->delim_end) { seterr(d, "Submessage ended in the middle of a value or group"); @@ -296,7 +296,7 @@ static NOINLINE size_t peekbytes_slow(upb_pbdecoder *d, void *buf, memcpy(buf, d->ptr, ret); if (in_residual_buf(d, d->ptr)) { size_t copy = UPB_MIN(bytes - ret, d->size_param); - memcpy(buf + ret, d->buf_param, copy); + memcpy((char *)buf + ret, d->buf_param, copy); ret += copy; } return ret; diff --git a/upb/pb/encoder.c b/upb/pb/encoder.c index d5685dc0ff..2534a4d617 100644 --- a/upb/pb/encoder.c +++ b/upb/pb/encoder.c @@ -79,7 +79,7 @@ static upb_pb_encoder_segment *top(upb_pb_encoder *e) { // Call to ensure that at least "bytes" bytes are available for writing at // e->ptr. Returns false if the bytes could not be allocated. static bool reserve(upb_pb_encoder *e, size_t bytes) { - if ((e->limit - e->ptr) < bytes) { + if ((size_t)(e->limit - e->ptr) < bytes) { size_t needed = bytes + (e->ptr - e->buf); size_t old_size = e->limit - e->buf; size_t new_size = old_size; @@ -110,7 +110,7 @@ static bool reserve(upb_pb_encoder *e, size_t bytes) { // Call when "bytes" bytes have been writte at e->ptr. The caller *must* have // previously called reserve() with at least this many bytes. static void encoder_advance(upb_pb_encoder *e, size_t bytes) { - assert((e->limit - e->ptr) >= bytes); + assert((size_t)(e->limit - e->ptr) >= bytes); e->ptr += bytes; } diff --git a/upb/table.c b/upb/table.c index fc69125c8a..9914a03347 100644 --- a/upb/table.c +++ b/upb/table.c @@ -542,7 +542,7 @@ void upb_inttable_compact(upb_inttable *t) { counts[log2ceil(key)]++; } - int arr_size; + size_t arr_size = 1; int arr_count = upb_inttable_count(t); if (upb_inttable_count(t) >= max_key * MIN_DENSITY) {