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;