threads / patch / 46799

patch, 4 partsavoid some -Wsign-compare warnings

Subject: [PATCH 0/4] avoid some -Wsign-compare warnings

## tl;dr

One message between Sep 21, 2017 and Sep 21, 2017. Diffs are folded; open one to read it.

replies: 0people: 1as markdown or json

Ramsay Jones· Sep 21, 2017, 16:45 UTC · lore

This series removes 687 '-Wsign-compare' warnings when applied to the current master branch (@ commit 59c0ea183). Using gcc v5.4.0 and adding '-Wextra' to the compilation flags, we can summarize the warnings as follows:

  $ grep warning out1 | sed -e 's/.*\[/\[/' | sort | uniq -c | sort -rn
     1437 [-Wunused-parameter]
     1410 [-Wsign-compare]
      670 [-Wmissing-field-initializers]
        7 [-Wempty-body]
  $ 
After applying this series, we see:
  $ grep warning out2 | sed -e 's/.*\[/\[/' | sort | uniq -c | sort -rn
     1437 [-Wunused-parameter]
      723 [-Wsign-compare]
      670 [-Wmissing-field-initializers]
        7 [-Wempty-body]
  $ 

The number of -Wunused-parameter warnings is not as bad as it seems; for example, we can get rid of 690 such warning with the following patch:

  $ git diff
  diff --git a/git-compat-util.h b/git-compat-util.h
  index cedad4d58..8e7388082 100644
  --- a/git-compat-util.h
  +++ b/git-compat-util.h
  @@ -118,6 +118,9 @@
   /* Approximation of the length of the decimal representation of this type. */
   #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
   
  +/* suppress 'unused parameter' warnings */
  +#define UNUSED(x) ((void)(x))
  +
   #if defined(__sun__)
    /*
     * On Solaris, when _XOPEN_EXTENDED is set, its header file
  @@ -341,6 +344,7 @@ typedef uintmax_t timestamp_t;
   #ifndef has_dos_drive_prefix
   static inline int git_has_dos_drive_prefix(const char *path)
   {
  +	UNUSED(path);
   	return 0;
   }
   #define has_dos_drive_prefix git_has_dos_drive_prefix
  @@ -349,6 +353,7 @@ static inline int git_has_dos_drive_prefix(const char *path)
   #ifndef skip_dos_drive_prefix
   static inline int git_skip_dos_drive_prefix(char **path)
   {
  +	UNUSED(path);
   	return 0;
   }
   #define skip_dos_drive_prefix git_skip_dos_drive_prefix
  $ 
  
  $ grep warning out3 | sed -e 's/.*\[/\[/' | sort | uniq -c | sort -rn
      747 [-Wunused-parameter]
      723 [-Wsign-compare]
      670 [-Wmissing-field-initializers]
        7 [-Wempty-body]
  $ 

The original version of the UNUSED macro used the gcc __unused__ attribute, but that caused some issues with msvc IIRC, so this version has been simplified. I have been meaning to check that this does not cause any bloat in the git executable (with all optimisation levels), so I haven't submitted it before. What do you think?

These patches reduce the error count quite a bit, without touching too many files, but additional patches may have to be batched up and submitted over several releases, viz:

  $ grep warning out2 | cut -d: -f1 | sort | uniq | wc -l
  264
  $ 
Ramsay Jones (4):
  git-compat-util.h: xsize_t() - avoid -Wsign-compare warnings
  commit-slab.h: avoid -Wsign-compare warnings
  cache.h: hex2chr() - avoid -Wsign-compare warnings
  ALLOC_GROW: avoid -Wsign-compare warnings
 builtin/pack-objects.c |  4 ++--
 cache.h                |  4 ++--
 commit-slab.h          |  6 +++---
 config.c               |  2 +-
 diff.c                 |  2 +-
 git-compat-util.h      |  6 ++++--
 line-log.c             | 18 +++++++++---------
 line-log.h             |  2 +-
 revision.c             |  2 +-
 tree-walk.c            |  3 +--
 10 files changed, 25 insertions(+), 24 deletions(-)
-- 
2.14.0

← back to recent threads