How to fix compiler warning

Discussion in 'C Programming' started by Dave Stafford, Aug 24, 2007.

  1. I have a macro that I use across the board for freeing ram. I'd like to
    clean up my code so I don't get these warnings.

    #define sfree(x) _internal_sfree((void **)&x)
    #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })

    void main() {
    char *x = (char *) malloc(10);
    int *y = (int *) malloc(10);

    sfree(x);
    sfree(y);
    }

    results in:

    warning: dereferencing type-punned pointer will break strict-aliasing
    rules
     
    Dave Stafford, Aug 24, 2007
    #1
    1. Advertising

  2. Dave Stafford

    Ian Collins Guest

    Dave Stafford wrote:
    > I have a macro that I use across the board for freeing ram. I'd like to
    > clean up my code so I don't get these warnings.
    >
    > #define sfree(x) _internal_sfree((void **)&x)
    > #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })
    >

    Remove the extraneous parenthesis around the expression.

    > void main() {


    If you didn't get a warning for this, crank up the warning level!

    --
    Ian Collins.
     
    Ian Collins, Aug 24, 2007
    #2
    1. Advertising

  3. Dave Stafford

    Ian Collins Guest

    Ian Collins wrote:
    > Dave Stafford wrote:
    >> I have a macro that I use across the board for freeing ram. I'd like to
    >> clean up my code so I don't get these warnings.
    >>
    >> #define sfree(x) _internal_sfree((void **)&x)
    >> #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })
    >>

    > Remove the extraneous parenthesis around the expression.
    >

    Which still leaves the question why cast to void** and why test for NULL?

    How about:

    #define sfree(x) _internal_sfree(&x)
    #define _internal_sfree(x) { free(*x); *x=NULL; }

    --
    Ian Collins.
     
    Ian Collins, Aug 24, 2007
    #3
  4. Dave Stafford <> writes:
    > I have a macro that I use across the board for freeing ram. I'd like to
    > clean up my code so I don't get these warnings.
    >
    > #define sfree(x) _internal_sfree((void **)&x)
    > #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })
    >
    > void main() {
    > char *x = (char *) malloc(10);
    > int *y = (int *) malloc(10);
    >
    > sfree(x);
    > sfree(y);
    > }
    >
    > results in:
    >
    > warning: dereferencing type-punned pointer will break strict-aliasing
    > rules


    Your macro depends on a gcc-specific extension (see "Statement Exprs"
    in the gcc manual).

    You take care to avoid passing a null pointer to free(), but
    free(NULL) is guaranteed to do nothing.

    Take a look at the following version of your program.
    ================================
    #include <stdlib.h>

    #define SFREE(p) (free(p), (p) = NULL)

    int main(void) {
    char *x = malloc(10);
    int *y = malloc(10 * sizeof *y);

    SFREE(x);
    SFREE(y);
    return 0;
    }
    ================================

    Every change I made fixes a bug in your code:

    main() returns int, not void. And since it returns int, you should
    return an int.

    I renamed the macro from "sfree" to "SFREE"; by convention, most
    macros should have all-caps names.

    In a macro definition, references to arguments should be enclosed in
    parentheses to avoid operator precedence problems.

    Casting the result of malloc() is useless, and can hide bugs.

    You didn't have a '#include <stdlib.h>'. This is required if you're
    going to call malloc(). The casts probably silenced your compiler's
    warning about this, but didn't fix the bug (it's like snipping the
    wire to a warning light on your car's dashboard).

    Allocating 10 bytes for an int* doesn't make much sense if, for
    example, sizeof(int) == 4. I changed it to allocate 10 ints.

    Recommended reading: the comp.lang.c FAQ, <http://www.c-faq.com/>.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Aug 25, 2007
    #4
  5. Ian Collins wrote:
    > Ian Collins wrote:
    >> Dave Stafford wrote:
    >>> I have a macro that I use across the board for freeing ram. I'd like to
    >>> clean up my code so I don't get these warnings.
    >>>
    >>> #define sfree(x) _internal_sfree((void **)&x)
    >>> #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })
    >>>

    >> Remove the extraneous parenthesis around the expression.
    >>

    > Which still leaves the question why cast to void** and why test for NULL?
    >
    > How about:
    >
    > #define sfree(x) _internal_sfree(&x)
    > #define _internal_sfree(x) { free(*x); *x=NULL; }


    Surely you should check that x!=NULL before calling *any* function on *x?

    --
    Philip Potter pgp <at> doc.ic.ac.uk
     
    Philip Potter, Aug 25, 2007
    #5
  6. Dave Stafford

    Ian Collins Guest

    Philip Potter wrote:
    > Ian Collins wrote:
    >> Ian Collins wrote:
    >>> Dave Stafford wrote:
    >>>> I have a macro that I use across the board for freeing ram. I'd
    >>>> like to
    >>>> clean up my code so I don't get these warnings.
    >>>>
    >>>> #define sfree(x) _internal_sfree((void **)&x)
    >>>> #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })
    >>>>
    >>> Remove the extraneous parenthesis around the expression.
    >>>

    >> Which still leaves the question why cast to void** and why test for NULL?
    >>
    >> How about:
    >>
    >> #define sfree(x) _internal_sfree(&x)
    >> #define _internal_sfree(x) { free(*x); *x=NULL; }

    >
    > Surely you should check that x!=NULL before calling *any* function on *x?
    >

    Good point! The noise got in the way...

    --
    Ian Collins.
     
    Ian Collins, Aug 25, 2007
    #6
  7. Dave Stafford wrote:
    > I have a macro that I use across the board for freeing ram. I'd like to
    > clean up my code so I don't get these warnings.


    What about those diagnostics you _should_ have gotten, but either didn't
    get or forgot to tell us about?

    You seem to have already taken the first step toward getting rid of the
    diagnostics: you have set your diagnostic level too low. Lower it again
    and maybe it will compile Fortran as C without complaint.

    >
    > #define sfree(x) _internal_sfree((void **)&x)
    > #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })
    >
    > void main() {
    > char *x = (char *) malloc(10);
    > int *y = (int *) malloc(10);
    >
    > sfree(x);
    > sfree(y);
    > }
    >
    > results in:
    >
    > warning: dereferencing type-punned pointer will break strict-aliasing
    > rules
    >
     
    Martin Ambuhl, Aug 25, 2007
    #7
  8. Philip Potter <> writes:

    > Ian Collins wrote:
    >> Ian Collins wrote:
    >>> Dave Stafford wrote:
    >>>> I have a macro that I use across the board for freeing ram. I'd like to
    >>>> clean up my code so I don't get these warnings.
    >>>>
    >>>> #define sfree(x) _internal_sfree((void **)&x)
    >>>> #define _internal_sfree(x) ({ if(x && *x) { free(*x); *x=NULL; } })
    >>>>
    >>> Remove the extraneous parenthesis around the expression.
    >>>

    >> Which still leaves the question why cast to void** and why test for NULL?
    >>
    >> How about:
    >>
    >> #define sfree(x) _internal_sfree(&x)
    >> #define _internal_sfree(x) { free(*x); *x=NULL; }

    >
    > Surely you should check that x!=NULL before calling *any* function
    > on *x?


    That is not really a problem -- the x in _internal_sfree is always of
    the form &... [Obviously this is only true if it is not invoked
    directly, but there is no practical way to avoid problems if internal
    helper macros are invoked by user code.]

    Since the x is of the form &... and * and & "cancel each other out"
    (talking very loosely) you end up not needing the other macro. With
    the required extra parentheses, a comma expression rather than a
    block, and a better name you get Keith's version:

    #define SFREE(p) (free(p), (p) = NULL)

    --
    Ben.
     
    Ben Bacarisse, Aug 25, 2007
    #8
  9. Ben Bacarisse wrote:
    > Philip Potter <> writes:
    >
    >> Ian Collins wrote:
    >>> Which still leaves the question why cast to void** and why test for NULL?
    >>>
    >>> How about:
    >>>
    >>> #define sfree(x) _internal_sfree(&x)
    >>> #define _internal_sfree(x) { free(*x); *x=NULL; }

    >> Surely you should check that x!=NULL before calling *any* function
    >> on *x?

    >
    > That is not really a problem -- the x in _internal_sfree is always of
    > the form &... [Obviously this is only true if it is not invoked
    > directly, but there is no practical way to avoid problems if internal
    > helper macros are invoked by user code.]


    Yes of course, I should have seen that before!



    --
    Philip Potter pgp <at> doc.ic.ac.uk
     
    Philip Potter, Aug 25, 2007
    #9
  10. Dave Stafford

    Ian Collins Guest

    Philip Potter wrote:
    > Ben Bacarisse wrote:
    >> Philip Potter <> writes:
    >>
    >>> Ian Collins wrote:
    >>>> Which still leaves the question why cast to void** and why test for
    >>>> NULL?
    >>>>
    >>>> How about:
    >>>>
    >>>> #define sfree(x) _internal_sfree(&x)
    >>>> #define _internal_sfree(x) { free(*x); *x=NULL; }
    >>> Surely you should check that x!=NULL before calling *any* function
    >>> on *x?

    >>
    >> That is not really a problem -- the x in _internal_sfree is always of
    >> the form &... [Obviously this is only true if it is not invoked
    >> directly, but there is no practical way to avoid problems if internal
    >> helper macros are invoked by user code.]

    >
    > Yes of course, I should have seen that before!
    >

    See what happens when "function like" macros have lower case names :)

    --
    Ian Collins.
     
    Ian Collins, Aug 25, 2007
    #10
  11. Ian Collins wrote:
    > Philip Potter wrote:
    >> Ben Bacarisse wrote:
    >>> Philip Potter <> writes:
    >>>
    >>>> Ian Collins wrote:
    >>>>> Which still leaves the question why cast to void** and why test for
    >>>>> NULL?
    >>>>>
    >>>>> How about:
    >>>>>
    >>>>> #define sfree(x) _internal_sfree(&x)
    >>>>> #define _internal_sfree(x) { free(*x); *x=NULL; }
    >>>> Surely you should check that x!=NULL before calling *any* function
    >>>> on *x?
    >>> That is not really a problem -- the x in _internal_sfree is always of
    >>> the form &... [Obviously this is only true if it is not invoked
    >>> directly, but there is no practical way to avoid problems if internal
    >>> helper macros are invoked by user code.]

    >> Yes of course, I should have seen that before!
    >>

    > See what happens when "function like" macros have lower case names :)
    >

    How would it be any different if they were real functions?

    --
    Philip Potter pgp <at> doc.ic.ac.uk
     
    Philip Potter, Aug 25, 2007
    #11
  12. Philip Potter <> writes:
    > Ian Collins wrote:
    >> Philip Potter wrote:
    >>> Ben Bacarisse wrote:
    >>>> Philip Potter <> writes:
    >>>>
    >>>>> Ian Collins wrote:
    >>>>>> Which still leaves the question why cast to void** and why test for
    >>>>>> NULL?
    >>>>>>
    >>>>>> How about:
    >>>>>>
    >>>>>> #define sfree(x) _internal_sfree(&x)
    >>>>>> #define _internal_sfree(x) { free(*x); *x=NULL; }
    >>>>> Surely you should check that x!=NULL before calling *any* function
    >>>>> on *x?
    >>>> That is not really a problem -- the x in _internal_sfree is always of
    >>>> the form &... [Obviously this is only true if it is not invoked
    >>>> directly, but there is no practical way to avoid problems if internal
    >>>> helper macros are invoked by user code.]
    >>> Yes of course, I should have seen that before!
    >>>

    >> See what happens when "function like" macros have lower case names :)
    >>

    > How would it be any different if they were real functions?


    The sfree() macro takes the address of its argument. If a function
    did that, it would get the address of the parameter, a local object,
    not the address of the object you pass to it.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Aug 25, 2007
    #12
  13. Ian Collins said:

    > Dave Stafford wrote:
    >
    >> void main() {

    >
    > If you didn't get a warning for this, crank up the warning level!


    Whilst it's wrong (except on some freestanding implementations), it
    isn't a syntax error or a constraint violation. Implementations need
    not diagnose it (although some choose to do so).

    --
    Richard Heathfield <http://www.cpax.org.uk>
    Email: -www. +rjh@
    Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
    "Usenet is a strange place" - dmr 29 July 1999
     
    Richard Heathfield, Aug 25, 2007
    #13
  14. Dave Stafford

    pete Guest

    Richard Heathfield wrote:
    >
    > Ian Collins said:
    >
    > > Dave Stafford wrote:
    > >
    > >> void main() {

    > >
    > > If you didn't get a warning for this, crank up the warning level!

    >
    > Whilst it's wrong (except on some freestanding implementations), it
    > isn't a syntax error or a constraint violation. Implementations need
    > not diagnose it (although some choose to do so).


    I don't see void main() as being as more or less correct
    on freestanding implementations which define void main(),
    than it is on hosted implementations which define void main().

    --
    pete
     
    pete, Aug 25, 2007
    #14
  15. Dave Stafford

    Ian Collins Guest

    pete wrote:
    > Richard Heathfield wrote:
    >> Ian Collins said:
    >>
    >>> Dave Stafford wrote:
    >>>
    >>>> void main() {
    >>> If you didn't get a warning for this, crank up the warning level!

    >> Whilst it's wrong (except on some freestanding implementations), it
    >> isn't a syntax error or a constraint violation. Implementations need
    >> not diagnose it (although some choose to do so).

    >
    > I don't see void main() as being as more or less correct
    > on freestanding implementations which define void main(),
    > than it is on hosted implementations which define void main().
    >

    Section 5.1.2.2.1 of the standard does.

    --
    Ian Collins.
     
    Ian Collins, Aug 25, 2007
    #15
  16. pete said:

    > Richard Heathfield wrote:
    >>
    >> Ian Collins said:
    >>
    >> > Dave Stafford wrote:
    >> >
    >> >> void main() {
    >> >
    >> > If you didn't get a warning for this, crank up the warning level!

    >>
    >> Whilst it's wrong (except on some freestanding implementations), it
    >> isn't a syntax error or a constraint violation. Implementations need
    >> not diagnose it (although some choose to do so).

    >
    > I don't see void main() as being as more or less correct
    > on freestanding implementations which define void main(),
    > than it is on hosted implementations which define void main().


    In C90, it's wrong on hosted implementations. Freestanding
    implementations are free to insist on their own entry point syntax if
    they wish, but C90 implementations must provide int main(int, char **)
    and correct C90 programs must use int main(int char **) - not to do so
    renders the behaviour of the program undefined.

    In C99, the situation has not changed in the freestanding world - and,
    in practice, it hasn't actually changed in the hosted world either,
    since implementations were always free to document alternate entry
    point syntax. The only difference is academic - in C99, if your program
    takes advantage of an implementation's documented alternative, your
    program's behaviour *under that implementation* is
    implementation-defined rather than undefined. If you try to port it,
    however, this academic distinction vanishes, and you're right back to
    undefined behaviour again. That change was one of C99's grosser
    stupidities.

    --
    Richard Heathfield <http://www.cpax.org.uk>
    Email: -www. +rjh@
    Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
    "Usenet is a strange place" - dmr 29 July 1999
     
    Richard Heathfield, Aug 25, 2007
    #16
  17. [comp.lang.c] Richard Heathfield <> wrote:

    > Ian Collins said:


    >> If you didn't get a warning for this, crank up the warning level!


    > Whilst it's wrong (except on some freestanding implementations), it
    > isn't a syntax error or a constraint violation. Implementations need
    > not diagnose it (although some choose to do so).


    I would think that an implementation worth its salt would provide a
    warning level at which it did issue a diagnostic for void main(). Are
    there any major implementations which cannot be coerced into doing so?

    --
    C. Benson Manica | I appreciate all corrections, polite or otherwise.
    cbmanica(at)gmail.com |
    ----------------------| I do not currently read any posts posted through
    sdf.lonestar.org | Google groups, due to rampant unchecked spam.
     
    Christopher Benson-Manica, Aug 25, 2007
    #17
  18. Christopher Benson-Manica said:

    > [comp.lang.c] Richard Heathfield <> wrote:
    >
    >> Ian Collins said:

    >
    >>> If you didn't get a warning for this, crank up the warning level!

    >
    >> Whilst it's wrong (except on some freestanding implementations), it
    >> isn't a syntax error or a constraint violation. Implementations need
    >> not diagnose it (although some choose to do so).

    >
    > I would think that an implementation worth its salt would provide a
    > warning level at which it did issue a diagnostic for void main(). Are
    > there any major implementations which cannot be coerced into doing so?


    Borland can, and gcc can. Last time I checked, which was VS6, Microsoft
    couldn't.

    --
    Richard Heathfield <http://www.cpax.org.uk>
    Email: -www. +rjh@
    Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
    "Usenet is a strange place" - dmr 29 July 1999
     
    Richard Heathfield, Aug 25, 2007
    #18
  19. Dave Stafford

    Guest

    On Aug 25, 12:45 pm, Richard Heathfield <> wrote:
    > pete said:
    >
    >
    >
    > > Richard Heathfield wrote:

    >
    > >> Ian Collins said:

    >
    > >> > Dave Stafford wrote:

    >
    > >> >> void main() {

    >
    > >> > If you didn't get a warning for this, crank up the warning level!

    >
    > >> Whilst it's wrong (except on some freestanding implementations), it
    > >> isn't a syntax error or a constraint violation. Implementations need
    > >> not diagnose it (although some choose to do so).

    >
    > > I don't see void main() as being as more or less correct
    > > on freestanding implementations which define void main(),
    > > than it is on hosted implementations which define void main().

    >
    > In C90, it's wrong on hosted implementations. Freestanding
    > implementations are free to insist on their own entry point syntax if
    > they wish, but C90 implementations must provide int main(int, char **)
    > and correct C90 programs must use int main(int char **) - not to do so
    > renders the behaviour of the program undefined.


    I believe you are mistaken - in both C90 and C99, main can also take
    no parameters. (It must still return an int of course.)

    > In C99, the situation has not changed in the freestanding world - and,
    > in practice, it hasn't actually changed in the hosted world either,
    > since implementations were always free to document alternate entry
    > point syntax. The only difference is academic - in C99, if your program
    > takes advantage of an implementation's documented alternative, your
    > program's behaviour *under that implementation* is
    > implementation-defined rather than undefined. If you try to port it,
    > however, this academic distinction vanishes, and you're right back to
    > undefined behaviour again. That change was one of C99's grosser
    > stupidities.
    >
    > --
    > Richard Heathfield <http://www.cpax.org.uk>
    > Email: -www. +rjh@
    > Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
    > "Usenet is a strange place" - dmr 29 July 1999
     
    , Aug 25, 2007
    #19
  20. said:

    > On Aug 25, 12:45 pm, Richard Heathfield <> wrote:
    >> [...] C90 implementations must provide int main(int, char
    >> **) and correct C90 programs must use int main(int char **) - not to
    >> do so renders the behaviour of the program undefined.

    >
    > I believe you are mistaken - in both C90 and C99, main can also take
    > no parameters. (It must still return an int of course.)


    Whoops! Yes, of course it can instead take no parameters. Would you
    believe me if I told you I knew that really? :)

    --
    Richard Heathfield <http://www.cpax.org.uk>
    Email: -www. +rjh@
    Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
    "Usenet is a strange place" - dmr 29 July 1999
     
    Richard Heathfield, Aug 25, 2007
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.

Share This Page