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 Add Comment Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but 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

Add a Patch

Pull Requests

Add a Pull Request

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-2024 The PHP Group
All rights reserved. 		Last updated: Fri Apr 19 07:01:27 2024 UTC