{"thread":{"id":"46799","subject":"[PATCH 0/4] avoid some -Wsign-compare warnings","startedAt":"2017-09-21T16:45:15Z","lastAt":"2017-09-21T16:45:15Z","messageCount":1,"participants":["Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"328574","messageId":"7c6dc899-fc98-e5cf-f16b-6b996a8dd6ef@ramsayjones.plus.com","threadId":"46799","inReplyTo":null,"subject":"[PATCH 0/4] avoid some -Wsign-compare warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-09-21T16:45:06Z","receivedAt":"2017-09-21T16:45:15Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nThis series removes 687 '-Wsign-compare' warnings when applied to the\ncurrent master branch (@ commit 59c0ea183). Using gcc v5.4.0 and adding\n'-Wextra' to the compilation flags, we can summarize the warnings as\nfollows:\n\n  $ grep warning out1 | sed -e 's/.*\\[/\\[/' | sort | uniq -c | sort -rn\n     1437 [-Wunused-parameter]\n     1410 [-Wsign-compare]\n      670 [-Wmissing-field-initializers]\n        7 [-Wempty-body]\n  $ \n\nAfter applying this series, we see:\n\n  $ grep warning out2 | sed -e 's/.*\\[/\\[/' | sort | uniq -c | sort -rn\n     1437 [-Wunused-parameter]\n      723 [-Wsign-compare]\n      670 [-Wmissing-field-initializers]\n        7 [-Wempty-body]\n  $ \n\nThe number of -Wunused-parameter warnings is not as bad as it seems; for\nexample, we can get rid of 690 such warning with the following patch:\n\n  $ git diff\n  diff --git a/git-compat-util.h b/git-compat-util.h\n  index cedad4d58..8e7388082 100644\n  --- a/git-compat-util.h\n  +++ b/git-compat-util.h\n  @@ -118,6 +118,9 @@\n   /* Approximation of the length of the decimal representation of this type. */\n   #define decimal_length(x)\t((int)(sizeof(x) * 2.56 + 0.5) + 1)\n   \n  +/* suppress 'unused parameter' warnings */\n  +#define UNUSED(x) ((void)(x))\n  +\n   #if defined(__sun__)\n    /*\n     * On Solaris, when _XOPEN_EXTENDED is set, its header file\n  @@ -341,6 +344,7 @@ typedef uintmax_t timestamp_t;\n   #ifndef has_dos_drive_prefix\n   static inline int git_has_dos_drive_prefix(const char *path)\n   {\n  +\tUNUSED(path);\n   \treturn 0;\n   }\n   #define has_dos_drive_prefix git_has_dos_drive_prefix\n  @@ -349,6 +353,7 @@ static inline int git_has_dos_drive_prefix(const char *path)\n   #ifndef skip_dos_drive_prefix\n   static inline int git_skip_dos_drive_prefix(char **path)\n   {\n  +\tUNUSED(path);\n   \treturn 0;\n   }\n   #define skip_dos_drive_prefix git_skip_dos_drive_prefix\n  $ \n  \n  $ grep warning out3 | sed -e 's/.*\\[/\\[/' | sort | uniq -c | sort -rn\n      747 [-Wunused-parameter]\n      723 [-Wsign-compare]\n      670 [-Wmissing-field-initializers]\n        7 [-Wempty-body]\n  $ \n\nThe original version of the UNUSED macro used the gcc __unused__ attribute,\nbut that caused some issues with msvc IIRC, so this version has been\nsimplified. I have been meaning to check that this does not cause any bloat\nin the git executable (with all optimisation levels), so I haven't submitted\nit before. What do you think?\n\nThese patches reduce the error count quite a bit, without touching too many\nfiles, but additional patches may have to be batched up and submitted over\nseveral releases, viz:\n\n  $ grep warning out2 | cut -d: -f1 | sort | uniq | wc -l\n  264\n  $ \n\nRamsay Jones (4):\n  git-compat-util.h: xsize_t() - avoid -Wsign-compare warnings\n  commit-slab.h: avoid -Wsign-compare warnings\n  cache.h: hex2chr() - avoid -Wsign-compare warnings\n  ALLOC_GROW: avoid -Wsign-compare warnings\n\n builtin/pack-objects.c |  4 ++--\n cache.h                |  4 ++--\n commit-slab.h          |  6 +++---\n config.c               |  2 +-\n diff.c                 |  2 +-\n git-compat-util.h      |  6 ++++--\n line-log.c             | 18 +++++++++---------\n line-log.h             |  2 +-\n revision.c             |  2 +-\n tree-walk.c            |  3 +--\n 10 files changed, 25 insertions(+), 24 deletions(-)\n\n-- \n2.14.0\n"}]}