Re: [PATCH gcc/fortran] get rid of gfc_free

2011-03-15 Thread Jim Meyering
Jakub Jelinek wrote:
 On Tue, Mar 15, 2011 at 11:02:38AM +0100, Jim Meyering wrote:
  Instead of please let us know, maybe recommend using
  __builtin_expect instead? E.g. something like
 
  if (__builtin_expect (ptr != NULL, 0))
  free (ptr);

 Good idea.  Thanks.
 Though how about avoiding the double negative?

if (__builtin_expect (ptr == NULL, 1))
  free (ptr);

 What double negative?  if (__builtin_expect (ptr != NULL, 0)) free (ptr);
 is certainly correct, the latter is wrong, it will leak memory.
 It will call free only if ptr is NULL, i.e. do a useless free (NULL),
 if it is non-NULL, it will not do anything.
 You could do
   if (!__builtin_expect (ptr == NULL, 1))
 free (ptr);
 but that doesn't seem to be nicer or clearer than
   if (__builtin_expect (ptr != NULL, 0))
 free (ptr);

Thanks for the quick correction.
I've fixed it locally, too.


Re: [PATCH gcc/fortran] get rid of gfc_free

2011-03-15 Thread Jim Meyering
Janne Blomqvist wrote:
...
 Hi Janne,

 These requested changes are in addition to (and independent of)
 the changes that I've already posted here.

 Yes, it was perhaps a bit unreasonable to ask you to fix this. OTOH
 with your changes gfc_free() was just a wrapper around free() and
 should thus be removed as unnecessary. Also, I believe this proper
 fix is more in the spirit of the request by Tobias and the message he
 linked to discussing the removal of gfc_free().

 The first cset below
 does your #2 and #3, and the second does #1.  I separate them for
 review because #1 is completely mechanical, while the others
 are manual.  You may prefer to combine them before pushing, for
 bisection.  Let me know if you'd prefer I submit in that form.

 All 3 changesets are ok for 4.7.

 I think it's fine to commit them separately if you prefer. If so,
 preferably in the order #3, #1, #2 in order to keep every revision
 buildable.

 Thanks for working on this!

Just so we're clear...
Currently while I do have a sourceware account,
I'm not in the gcc group, so don't have commit access,

sourceware$ id -a|grep gcc
[Exit 1]

so someone else would have to commit my changes.
Or add me to the gcc group and I will do it.

Another recently-approved change may be in limbo for this reason:

avoid memory overrun in a test leading to potential double-free
* testsuite/test-expandargv.c (writeout_test): Fix off-by-one error:
i.e., do copy the trailing NUL byte.