[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH] fix warning and possible invalid free
From: |
Tim Ruehsen |
Subject: |
Re: [Bug-wget] [PATCH] fix warning and possible invalid free |
Date: |
Mon, 16 Apr 2012 10:23:54 +0200 |
User-agent: |
KMail/1.13.7 (Linux/3.2.0-2-amd64; KDE/4.7.4; x86_64; ; ) |
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