Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > How to fix compiler warning

Reply
Thread Tools

How to fix compiler warning

 
 
Dave Stafford
Guest
Posts: n/a
 
      08-24-2007
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

 
Reply With Quote
 
 
 
 
Ian Collins
Guest
Posts: n/a
 
      08-24-2007
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.
 
Reply With Quote
 
 
 
 
Ian Collins
Guest
Posts: n/a
 
      08-24-2007
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.
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      08-24-2007
Dave Stafford <(E-Mail Removed)> 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.velocityreviews.com/forums/(E-Mail Removed) <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"
 
Reply With Quote
 
Philip Potter
Guest
Posts: n/a
 
      08-24-2007
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
 
Reply With Quote
 
Ian Collins
Guest
Posts: n/a
 
      08-24-2007
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.
 
Reply With Quote
 
Martin Ambuhl
Guest
Posts: n/a
 
      08-24-2007
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
>

 
Reply With Quote
 
Ben Bacarisse
Guest
Posts: n/a
 
      08-24-2007
Philip Potter <(E-Mail Removed)> 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.
 
Reply With Quote
 
Philip Potter
Guest
Posts: n/a
 
      08-25-2007
Ben Bacarisse wrote:
> Philip Potter <(E-Mail Removed)> 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
 
Reply With Quote
 
Ian Collins
Guest
Posts: n/a
 
      08-25-2007
Philip Potter wrote:
> Ben Bacarisse wrote:
>> Philip Potter <(E-Mail Removed)> 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.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Re: How include a large array? Edward A. Falk C Programming 1 04-04-2013 08:07 PM
Xah's Edu Corner: The Concepts and Confusions of Pre-fix, In-fix, Post-fix and Fully Functional Notations Xah Lee Perl Misc 21 03-21-2006 07:02 AM
Xah's Edu Corner: The Concepts and Confusions of Pre-fix, In-fix, Post-fix and Fully Functional Notations Xah Lee Python 23 03-21-2006 07:02 AM
Xah's Edu Corner: The Concepts and Confusions of Pre-fix, In-fix, Post-fix and Fully Functional Notations Xah Lee Java 22 03-21-2006 07:02 AM
Re: A code fix for MSVC warning C4267 (64-bit compatibility warning,e.g. Boost Spirit) Pete Becker C++ 0 02-10-2005 01:13 PM



Advertisments