php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: JClawson at tamu dot edu
New email:
PHP Version: OS:

 

 [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: Sat Jul 12 05:01:33 2025 UTC