git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ramsay Jones <ramsay@ramsayjones.plus.com>
Date
Sep 21, 2017, 16:45 UTC
Message-ID
<7c6dc899-fc98-e5cf-f16b-6b996a8dd6ef@ramsayjones.plus.com>

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
Message 1 of 1 in “avoid some -Wsign-compare warnings”
  1. 0/4 avoid some -Wsign-compare warningsRamsay Jones, Sep 21, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.