php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #32797 Compile fails with xml_element.c: my_free using gcc 4
Submitted: 2005-04-22 00:05 UTC Modified: 2005-04-22 17:53 UTC
From: JClawson at tamu dot edu Assigned:
Status: Closed Package: Compile Failure
PHP Version: 5.0.4 OS: * (with GCC 4 only)
Private report: No CVE-ID: None
View Developer Edit
 [2005-04-22 00:05 UTC] JClawson at tamu dot edu 
Description:
------------
The my_free function macro in ext/xmlrpc/libxmlrpc/xml_element.c is poorly written and results in invalid C code on line 192.  Please remove this macro completly from the file and write out the proper code for each case the free() function is called.

/home/upgrade-tmp/php5-STABLE-200504211236/ext/xmlrpc/libxmlrpc/xml_element.c: In function 'xml_elem_free_non_recurse':
/home/upgrade-tmp/php5-STABLE-200504211236/ext/xmlrpc/libxmlrpc/xml_element.c:192: error: invalid lvalue in assignment
/home/upgrade-tmp/php5-STABLE-200504211236/ext/xmlrpc/libxmlrpc/xml_element.c: In function 'xml_elem_entity_escape':
/home/upgrade-tmp/php5-STABLE-200504211236/ext/xmlrpc/libxmlrpc/xml_element.c:317: warning: pointer targets in assignment differ in signedness
/home/upgrade-tmp/php5-STABLE-200504211236/ext/xmlrpc/libxmlrpc/xml_element.c:332: warning: pointer targets in assignment differ in signedness
make: *** [ext/xmlrpc/libxmlrpc/xml_element.lo] Error 1


Reproduce code:
---------------
Here is the macro

#define my_free(thing)  if(thing) {free(thing); thing = 0;}


This will not work on 192:

my_free((char*)root->name);


Expected result:
----------------
Consider changing the call on 192 from:
my_free((char*)root->name);

to:

if(root -> name)
{
      free((char*)root->name);
      root->name = 0;
}

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2005-04-22 00:10 UTC] sniper@php.net 
Works fine with GCC 3.
 [2005-04-22 03:59 UTC] JClawson at tamu dot edu 
Sorry... the code I posted should be this

if (root->name) {
             free((char *)root->name);
             root->name = NULL;
      }
 [2005-04-22 04:10 UTC] JClawson at tamu dot edu 
Oh... Everyone will not be using GCC 3 forever.  Don't you think it would be prudent to correct obvious errors now?

After all if you have the following code:

if((char*)root->name)
{
   free((char*)root->name);
   (char*)root->name = 0;

}

why would you assign a pointer to 0?

And for whatever reason... why the stupid if statement????
Why not just simplify everything with:

free((char*)root->name);

Bam... correct C code.  If root->name is NULL thats ok... because free can take NULL as a paramater!

You don't free somthing and then try to assign an integer to it... seriously.
 [2005-04-22 04:44 UTC] wez@php.net 
Please lose the attitude; we don't have time for that.
As for PHP; you appear to have the knowledge--submit a working patch that doesn't break the code.
 [2005-04-22 07:24 UTC] abies@php.net 
1) Relying on free() to ignore NULL pointers is not portable: we do have to support other compilers beside GCC 4

2) Using cast expressions as lvalues has always been supported in C, but apparently GCC 4 suddenly doesn't support it anymore. In this particular case, however, we might change the macro so it only does the cast for the argument to free() , and only for compilers that require it, i.e. compilers/libc's that define free like free(char*)
 [2005-04-22 08:14 UTC] JClawson at tamu dot edu 
Hmmm... I thought free was supposed to ignore the null pointer.  At least that was my understanding:

Here is a nice table:
http://developer.apple.com/qa/qa2001/qa1259.html

I guess its better to be safe than sorry.
 [2005-04-22 09:14 UTC] sniper@php.net 
Not everyone will use GCC..ever.
 [2005-04-22 13:12 UTC] jorton@php.net 
This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

This is invalid C code; allowing cast-as-lvalue is an extension to C which has been deprecated (and has triggered warnings) since gcc 3.4.

Fixed on HEAD, will backport shortly.
 [2005-04-22 17:53 UTC] JClawson at tamu dot edu 
GREAT! Thanks!
 
PHP Copyright © 2001-2025 The PHP Group
All rights reserved. 		Last updated: Fri Nov 21 23:00:01 2025 UTC