Re: [PATCH gcc/fortran] get rid of gfc_free
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
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.