Re: [Bug-wget] [PATCH] fix warning and possible invalid free

[Tim Ruehsen]

Am Friday 13 April 2012 schrieb Micah Cowan:
> On 04/13/2012 01:44 AM, Tim Ruehsen wrote:
> > Am Thursday 12 April 2012 schrieb Micah Cowan:
> >> On 04/12/2012 01:23 AM, TeeRasen wrote:
> >>> In main.c we have
> >>> 
> >>>           opt.progress_type = "dot";
> >> 
> >> In C, a string literal is of type char[] (which automatically transforms
> >> to char*), not const char[] or const char* (even though one must still
> >> not modify it. You're either compiling with C++ (a bad idea for wget
> >> code), or a nonstandard C invocation that makes string literals out to
> >> be const. (Though, a C implementation is allowed to "warn" about
> >> whatever it wishes, provided it still behaves properly, I suppose.)
> > 
> > Maybe, my posting was too long to read...
> > The main message was that free("dot") crashes, since literals are
> > read-only.
> 
> Right; but free("dot") crashing has nothing to do with literals being
> read-only, and everything to do with literals not having been malloc()'d.
> 
> > Example (compile with gcc x.c -o x):
> > #include <stdlib.h>
> > void main(void)
> > {
> > 
> >         char *buf="dot";
> >         free(buf);
> > 
> > }
> > free crashes though gcc did not complain !
> > 
> > To make gcc check for these potential crashes, use -Wwrite-strings.
> 
> -Wwrite-strings doesn't do anything at all to indicate the real problem,
> which is the free(). If you change "char *buf" to "const char *buf",
> -Wwrite-strings is satisfied, and yet the free() is still just as
> dangerous as ever.
> 
> If opt.progress_type were const char * (which would probably not be a
> terrible idea), the bug would still remain, and -Wwrite-strings wouldn't
> say anything about it. So I was just trying to point out that the
> warning you mentioned never had anything to do with the real bug, and
> doesn't actually indicate a problem (there's nothing wrong with
> assigning literals to non-const pointers, provided you never actually
> try to modify them). Perhaps it's tangential to the main point of your
> email, but I thought it was worth pointing out.

You are right (now I got your intention, sorry).

BTW, I just found the (potential) free bug because I investigated into the 
'const' warning.

-Wwrite-strings is just a tool, which let's one find mixed malloc/literal 
assignments (besides other kinds of flaws). In such a case:
* either the programmer has some kind of 'release_policy' flag as found in 
http.c (which takes away control from the compiler and burdens the programmer)
* or a free is missing (potential memleak)
* or free is NOT missing (potential freeing of literal).

Away from that, using 'const' where possible helps finding resp. helps 
avoiding potential crashes (writing into read-only memory). It gives the 
compiler more 'insight', thus reduces the programmer's burden of keeping 
everything in mind.

So, turning opt.progress_type and many other into 'const char *' is on my 
list.

> -mjc

     Tim