<<< Date Index >>>     <<< Thread Index >>>

Re: [PATCH] Compilation warnings, configure



On Tuesday, 03 April 2007 at 19:38, Fabian Groffen wrote:
> Hi,
> 
> I believe Brendan already fixed the headercache segfault, but I post my
> anti-warning patches anyway.  I believe it's good to have warningless
> code.  I attach two patches here.
> 
> configure.patch
>   might be useful in the future, enables X_CFLAGS to be recognised and
>   added to CFLAGS after configure has done its tests

what's X_CFLAGS and why should we worry about it? If the user can't
override CFLAGS I think that may be a bug.

> warnings.patch
>   warnings I fixed that GCC chokes on using -Werror -std=gnu99.  Because
>   I don't know how to solve the "%lld is forbidden in C90" error I used
>   the latter option.  Please review carefully.  The "may be used
>   unitialised" might actually really happen in some cases.

I'm a little afraid of -std=gnu99. Maybe we can flip that on after 1.6.

> diff -ur mutt-1.5.14cvs.orig/copy.c mutt-1.5.14cvs/copy.c
> 
> warning: XXX may be used initialised
> 
> +++ mutt-1.5.14cvs/copy.c     2007-04-03 19:07:58.526065000 +0200
> @@ -56,7 +56,7 @@
>    int hdr_count;
>    int x;
>    char *this_one = NULL;
> -  size_t this_one_len;
> +  size_t this_one_len = 0;
>    int error;
>  
>    if (ftello (in) != off_start)

I need to inspect that more closely -- it isn't obvious in two seconds
to me that this isn't a real bug.

> warning: dereferencing a type-punned variable will break strict aliasing
> rules

I don't know about these type-punning hacks. I think we need a better
fix...

> +++ mutt-1.5.14cvs/imap/message.c     2007-04-03 18:47:36.410996000 +0200
> @@ -80,6 +80,7 @@
>    unsigned int *uidnext = NULL;
>    int evalhc = 0;
>    char uid_buf[64];
> +  void *d;
>  #endif /* USE_HCACHE */
>  
>    ctx = idata->ctx;
> @@ -157,7 +158,8 @@
>          rc = imap_cmd_step (idata);
>          if (rc != IMAP_CMD_CONTINUE)
>       {
> -       imap_free_header_data ((void**) &h.data);
> +       d = &h.data;
> +       imap_free_header_data (&d);
>            break;
>       }
>  
> @@ -165,7 +167,8 @@
>            continue;
>          else if (mfhrc < 0)
>       {
> -       imap_free_header_data ((void**) &h.data);
> +       d = &h.data;
> +       imap_free_header_data (&d);
>            break;
>       }
>  
> @@ -193,9 +196,12 @@
>            ctx->size += ctx->hdrs[idx]->content->length;
>          }
>       else
> +        {
>         /* bad header in the cache, we'll have to refetch.
>          * TODO: consider the possibility of a holey cache. */
> -          imap_free_header_data((void**) &h.data);
> +          d = &h.data;
> +          imap_free_header_data(&d);
> +        }
>    
>          FREE(&uid_validity);
>        }
> @@ -205,7 +211,10 @@
>        if ((mfhrc < -1) || ((rc != IMAP_CMD_CONTINUE) && (rc != IMAP_CMD_OK)))
>        {
>          if (h.data)
> -          imap_free_header_data ((void**) &h.data);
> +             {
> +          d = &h.data;
> +          imap_free_header_data (&d);
> +             }
>          fclose (fp);
>          mutt_hcache_close (hc);
>          return -1;
> @@ -308,7 +317,10 @@
>      if ((mfhrc < -1) || ((rc != IMAP_CMD_CONTINUE) && (rc != IMAP_CMD_OK)))
>      {
>        if (h.data)
> -        imap_free_header_data ((void**) &h.data);
> +       {
> +        d = &h.data;
> +        imap_free_header_data (&d);
> +       }
>        fclose (fp);
>  #if USE_HCACHE
>        mutt_hcache_close (hc);

> diff -ur mutt-1.5.14cvs.orig/muttlib.c mutt-1.5.14cvs/muttlib.c
> 
> warning: XXX defined but never used

needs to be wrapped in #ifdef DEBUG actually. I've done that.

> second hunk: type-pun
> 
> +++ mutt-1.5.14cvs/muttlib.c  2007-04-03 19:11:39.717894000 +0200
> @@ -1029,7 +1029,6 @@
>      {
>        BUFFER *srcbuf, *word, *command;
>        char    srccopy[LONG_STRING];
> -      int     i = 0;
>  
>        dprint(3, (debugfile, "fmtpipe = %s\n", src));
>  
> @@ -1595,12 +1594,14 @@
>  void mutt_buffer_add (BUFFER* buf, const char* s, size_t len)
>  {
>    size_t offset;
> +  void *d;
>  
>    if (buf->dptr + len + 1 > buf->data + buf->dsize)
>    {
>      offset = buf->dptr - buf->data;
>      buf->dsize += len < 128 ? 128 : len + 1;
> -    safe_realloc ((void**) &buf->data, buf->dsize);
> +    d = &buf->data;
> +    safe_realloc (&d, buf->dsize);
>      buf->dptr = buf->data + offset;
>    }
>    memcpy (buf->dptr, s, len);
> diff -ur mutt-1.5.14cvs.orig/regex.c mutt-1.5.14cvs/regex.c
> 
> warning: comparison always yields in false due to limited data type
> second hunk: warning: XXX defined but never used

as you noted, this is wrong.

> +++ mutt-1.5.14cvs/regex.c    2007-04-03 19:14:11.980070000 +0200
> @@ -2197,7 +2197,7 @@
>                        {
>                          PATFETCH (c);
>                          if (c == ':' || c == ']' || p == pend
> -                            || c1 == CHAR_CLASS_MAX_LENGTH)
> +                            || c1 == (unsigned char) CHAR_CLASS_MAX_LENGTH)
>                            break;
>                          str[c1++] = c;
>                        }
> @@ -3117,9 +3117,6 @@
>  #ifndef REGEX_MALLOC
>    char *destination;
>  #endif
> -  /* We don't push any register information onto the failure stack.  */
> -  unsigned num_regs = 0;
> -
>    register char *fastmap = bufp->fastmap;
>    unsigned char *pattern = bufp->buffer;
>    unsigned char *p = pattern;