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

Re: [PATCH] Compilation warnings, configure



On 03-04-2007 11:03:44 -0700, Brendan Cully wrote:
> > 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.

configure can't handle -Werror.  It will die during the tests.  So as
developer you might want to have an easy way to add it, without having
to edit all generated Makefiles afterwards every time.

> > 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.

I didn't add -std=gnu99 in the patch, I just used it to ignore the %lld
formatting warning being treated as errors.

> > 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.

Good.  I noticed I was a bit to quick.  On Darwin there are more of
these, and I broke something.  I'm sorry about that.

        if (flags & CH_DECODE)
        {
          if (!address_header_decode (&this_one))
            rfc2047_decode (&this_one);
>          this_one_len = mutt_strlen (this_one);
        }

        if (!headers[x])
          headers[x] = this_one;
        else
        {
          int hlen = mutt_strlen (headers[x]);

>          safe_realloc (&headers[x], hlen + this_one_len + sizeof (char));

Theoretically, flags & CH_DECODE != true, and headers[x] == true, using
this_one_len without being set.

> > 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...

I agree it isn't elegant.  The warning more or less says pointers are being 
casted, not the values.  Problem here is that the pointer type might
not match the allocated data size.

-- 
Fabian Groffen
Gentoo on a different level