Re: [PATCH] maint: avoid new coverity warnings

[Bernhard Voelker]

On 05/29/2015 07:39 PM, Pádraig Brady wrote:
> * src/sync.c (sync_arg): Initialise variable to avoid
> unitialized access if assert is disabled.
> * src/head.c (elide_tail_bytes_file): Support this function
> with ---presume-input-pipe and larger files,
> which regressed with commit v8.23-47-g2662702.
> (elide_tail_lines_file): Likewise.
> * src/dd.c (dd_copy): Explicitly don't try to ftruncate()
> upon failure to lseek() (the existing check against
> st_size was already protecting that).
> * src/factor.c (factor_using_squfof): Assert (only when
> linting due to performance) to avoid the implication of
> divide by zero.
> * src/od.c (read_block): Remove dead code.
> * src/tac.c (tac_seekable): Likewise.
> ---
>  src/dd.c     | 2 +-
>  src/factor.c | 3 ++-
>  src/head.c   | 4 ++--
>  src/od.c     | 3 ---
>  src/sync.c   | 2 +-
>  src/tac.c    | 3 ---
>  6 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/src/dd.c b/src/dd.c
> index 6b09bc6..e647294 100644
> --- a/src/dd.c
> +++ b/src/dd.c
> @@ -2280,7 +2280,7 @@ dd_copy (void)
>        if (S_ISREG (stdout_stat.st_mode) || S_TYPEISSHM (&stdout_stat))
>          {
>            off_t output_offset = lseek (STDOUT_FILENO, 0, SEEK_CUR);
> -          if (output_offset > stdout_stat.st_size)
> +          if (0 <= output_offset && stdout_stat.st_size < output_offset)
>              {
>                if (iftruncate (STDOUT_FILENO, output_offset) != 0)
>                  {
> diff --git a/src/factor.c b/src/factor.c
> index 4e4d0c7..f27bf22 100644
> --- a/src/factor.c
> +++ b/src/factor.c
> @@ -2055,8 +2055,9 @@ factor_using_squfof (uintmax_t n1, uintmax_t n0, struct 
> factors *factors)
>            div_smallq (q, rem, S+P, Q);
>            P1 = S - rem; /* P1 = q*Q - P */
>  
> +          IF_LINT (assert (q > 0 && Q > 0));
> +
>  #if STAT_SQUFOF
> -          assert (q > 0);
>            q_freq[0]++;
>            q_freq[MIN (q, Q_FREQ_SIZE)]++;
>  #endif
> diff --git a/src/head.c b/src/head.c
> index 3ea81b6..410cc4f 100644
> --- a/src/head.c
> +++ b/src/head.c
> @@ -458,7 +458,7 @@ elide_tail_bytes_file (const char *filename, int fd, 
> uintmax_t n_elide,
>                         struct stat const *st, off_t current_pos)
>  {
>    off_t size = st->st_size;
> -  if (size <= ST_BLKSIZE (*st))
> +  if (presume_input_pipe || size <= ST_BLKSIZE (*st))
>      return elide_tail_bytes_pipe (filename, fd, n_elide, current_pos);
>    else
>      {
> @@ -747,7 +747,7 @@ elide_tail_lines_file (const char *filename, int fd, 
> uintmax_t n_elide,
>                         struct stat const *st, off_t current_pos)
>  {
>    off_t size = st->st_size;
> -  if (size <= ST_BLKSIZE (*st))
> +  if (presume_input_pipe || size <= ST_BLKSIZE (*st))
>      return elide_tail_lines_pipe (filename, fd, n_elide, current_pos);
>    else
>      {
> diff --git a/src/od.c b/src/od.c
> index c1241f5..0ca3ca7 100644
> --- a/src/od.c
> +++ b/src/od.c
> @@ -1291,9 +1291,6 @@ read_block (size_t n, char *block, size_t 
> *n_bytes_in_buffer)
>  
>    *n_bytes_in_buffer = 0;
>  
> -  if (n == 0)
> -    return true;
> -
>    while (in_stream != NULL)  /* EOF.  */
>      {
>        size_t n_needed;
> diff --git a/src/sync.c b/src/sync.c
> index b71b8bc..85d77c0 100644
> --- a/src/sync.c
> +++ b/src/sync.c
> @@ -117,7 +117,7 @@ sync_arg (enum sync_mode mode, char const *file)
>  
>    if (ret == true)
>      {
> -      int sync_status;
> +      int sync_status = -1;
>  
>        switch (mode)
>          {
> diff --git a/src/tac.c b/src/tac.c
> index 2d73c6e..295ac77 100644
> --- a/src/tac.c
> +++ b/src/tac.c
> @@ -331,9 +331,6 @@ tac_seekable (int input_fd, const char *file, off_t 
> file_pos)
>                  xalloc_die ();
>                newbuffer = xrealloc (G_buffer - offset, G_buffer_size);
>                newbuffer += offset;
> -              /* Adjust the pointers for the new buffer location.  */
> -              match_start = newbuffer + match_start_offset;
> -              past_end = newbuffer + past_end_offset;
>                G_buffer = newbuffer;
>              }
>  
> 

Nice!
All but the last look good - the last results in a new unused variable
warning/error:

src/tac.c: In function 'tac_seekable':
src/tac.c:325:25: error: unused variable 'past_end_offset' 
[-Werror=unused-variable]
               ptrdiff_t past_end_offset = past_end - G_buffer;
                         ^
src/tac.c:324:25: error: unused variable 'match_start_offset' 
[-Werror=unused-variable]
               ptrdiff_t match_start_offset = match_start - G_buffer;
                         ^

BTW: you could squash in the following for another new coverity
warning to avoid numerical comparison with a bool:

diff --git a/src/dircolors.c b/src/dircolors.c
index 3a03f1f..6cf0cdb 100644
--- a/src/dircolors.c
+++ b/src/dircolors.c
@@ -442,7 +442,7 @@ main (int argc, char **argv)
       usage (EXIT_FAILURE);
     }

-  if ((!print_database) < argc)
+  if ((print_database ? 0 : 1) < argc)
     {
       error (0, 0, _("extra operand %s"), quote (argv[!print_database]));
       if (print_database)


Thanks & have a nice day,
Berny