Patchwork [v3] Re: avoid useless if-before-free tests

login
register
mail settings
Submitter Jim Meyering
Date March 24, 2011, 4:51 p.m.
Message ID <87ei5w8nt7.fsf@rho.meyering.net>
Download mbox | patch
Permalink /patch/88236/
State New
Headers show

Comments

Jim Meyering - March 24, 2011, 4:51 p.m.
Janne Blomqvist wrote:
> On Tue, Mar 8, 2011 at 19:53, Jim Meyering <jim@meyering.net> wrote:
>> Relative to v2, I've added libgo/ to the list of exempt directories and added
>> this recently discussed gfc_free patch, at the request of Tobias Burnus.
>> Also, I corrected an error in fortran's ChangeLog and removed all
>> whitespace changes from all ChangeLog files.
>
> The libgfortran changes are Ok for 4.7.
>
> For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd
>
> - Replace all calls to "gfc_free (x)" with "free (x)".
> - Remove the gfc_free() function and prototype.
> - Remove the free() macro which currently prevents calling free() directly.

Following up, I've refreshed the series but hit a minor snag
while converting new uses of gfc_free, removing new tests-before-free
and merging/reordering changes.

Applying this fix first makes my problem go away:

From 77142dc7da9e1e11ef8b0c554df4ff5c1bbdda39 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Thu, 24 Mar 2011 17:45:42 +0100
Subject: [PATCH] gfortran: remove unneeded test-before-gfc_free

* trans-openmp.c (gfc_trans_omp_reduction_list): Do not guard
use of gfc_free; it can handle a NULL argument.
---
 gcc/fortran/ChangeLog      |    5 +++++
 gcc/fortran/trans-openmp.c |    3 +--
 2 files changed, 6 insertions(+), 2 deletions(-)

--
1.7.4.1.686.g46300
Janne Blomqvist - April 15, 2011, 7:26 a.m.
On Thu, Mar 24, 2011 at 18:51, Jim Meyering <jim@meyering.net> wrote:
> Janne Blomqvist wrote:
>> On Tue, Mar 8, 2011 at 19:53, Jim Meyering <jim@meyering.net> wrote:
>>> Relative to v2, I've added libgo/ to the list of exempt directories and added
>>> this recently discussed gfc_free patch, at the request of Tobias Burnus.
>>> Also, I corrected an error in fortran's ChangeLog and removed all
>>> whitespace changes from all ChangeLog files.
>>
>> The libgfortran changes are Ok for 4.7.
>>
>> For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd
>>
>> - Replace all calls to "gfc_free (x)" with "free (x)".
>> - Remove the gfc_free() function and prototype.
>> - Remove the free() macro which currently prevents calling free() directly.
>
> Following up, I've refreshed the series but hit a minor snag
> while converting new uses of gfc_free, removing new tests-before-free
> and merging/reordering changes.
>
> Applying this fix first makes my problem go away:

[snip]

So, what's the plan here? Do you plan to get a GCC account, do you
already have one, or what? Now that 4.7 is open for development, it's
perhaps the right time to poke the maintainers to get this patch in.

If you don't have a GCC account, as one of the Fortran maintainers I
can commit the Fortran and libgfortran parts, but someone else will
have to do the rest (were they ever approved, BTW?) as I have only
write after approval privileges for the rest of GCC.
Jim Meyering - April 15, 2011, 7:54 a.m.
Janne Blomqvist wrote:

> On Thu, Mar 24, 2011 at 18:51, Jim Meyering <jim@meyering.net> wrote:
>> Janne Blomqvist wrote:
>>> On Tue, Mar 8, 2011 at 19:53, Jim Meyering <jim@meyering.net> wrote:
>>>> Relative to v2, I've added libgo/ to the list of exempt directories and added
>>>> this recently discussed gfc_free patch, at the request of Tobias Burnus.
>>>> Also, I corrected an error in fortran's ChangeLog and removed all
>>>> whitespace changes from all ChangeLog files.
>>>
>>> The libgfortran changes are Ok for 4.7.
>>>
>>> For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd
>>>
>>> - Replace all calls to "gfc_free (x)" with "free (x)".
>>> - Remove the gfc_free() function and prototype.
>>> - Remove the free() macro which currently prevents calling free() directly.
>>
>> Following up, I've refreshed the series but hit a minor snag
>> while converting new uses of gfc_free, removing new tests-before-free
>> and merging/reordering changes.
>>
>> Applying this fix first makes my problem go away:
>
> [snip]
>
> So, what's the plan here? Do you plan to get a GCC account, do you
> already have one, or what? Now that 4.7 is open for development, it's
> perhaps the right time to poke the maintainers to get this patch in.
>
> If you don't have a GCC account, as one of the Fortran maintainers I
> can commit the Fortran and libgfortran parts, but someone else will
> have to do the rest (were they ever approved, BTW?) as I have only
> write after approval privileges for the rest of GCC.

Can someone add me to the gcc group?  That would help.
I already have ssh access to sourceware.org.

I've rebased the series a few times, but it's been a week or so.
More convertible uses are being added regularly.
Plus I have to reorder/split things a little to avoid a new conflict.
Janne Blomqvist - April 15, 2011, 8:20 a.m.
On Fri, Apr 15, 2011 at 10:54, Jim Meyering <jim@meyering.net> wrote:
> Janne Blomqvist wrote:
>
>> On Thu, Mar 24, 2011 at 18:51, Jim Meyering <jim@meyering.net> wrote:
>>> Janne Blomqvist wrote:
>>>> On Tue, Mar 8, 2011 at 19:53, Jim Meyering <jim@meyering.net> wrote:
>>>>> Relative to v2, I've added libgo/ to the list of exempt directories and added
>>>>> this recently discussed gfc_free patch, at the request of Tobias Burnus.
>>>>> Also, I corrected an error in fortran's ChangeLog and removed all
>>>>> whitespace changes from all ChangeLog files.
>>>>
>>>> The libgfortran changes are Ok for 4.7.
>>>>
>>>> For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd
>>>>
>>>> - Replace all calls to "gfc_free (x)" with "free (x)".
>>>> - Remove the gfc_free() function and prototype.
>>>> - Remove the free() macro which currently prevents calling free() directly.
>>>
>>> Following up, I've refreshed the series but hit a minor snag
>>> while converting new uses of gfc_free, removing new tests-before-free
>>> and merging/reordering changes.
>>>
>>> Applying this fix first makes my problem go away:
>>
>> [snip]
>>
>> So, what's the plan here? Do you plan to get a GCC account, do you
>> already have one, or what? Now that 4.7 is open for development, it's
>> perhaps the right time to poke the maintainers to get this patch in.
>>
>> If you don't have a GCC account, as one of the Fortran maintainers I
>> can commit the Fortran and libgfortran parts, but someone else will
>> have to do the rest (were they ever approved, BTW?) as I have only
>> write after approval privileges for the rest of GCC.
>
> Can someone add me to the gcc group?  That would help.
> I already have ssh access to sourceware.org.

According to http://gcc.gnu.org/svnwrite.html , you should send an
email to overseers(at)gcc.gnu.org . It says that you need a sponsor
and "The steering committee or a well-established GCC maintainer
(including reviewers) can approve for write access any person with GNU
copyright assignment papers in place and known to submit good
patches.". I'm not sure if I'm considered to be well-established
enough, so could someone help Jim out here, please?

Or, if nobody pipes up within a few days, just mention my name as your
sponsor and we'll see if the overseers consider me well-established or
not.. ;)
Jim Meyering - April 15, 2011, 8:23 a.m.
Janne Blomqvist wrote:
> On Fri, Apr 15, 2011 at 10:54, Jim Meyering <jim@meyering.net> wrote:
>> Janne Blomqvist wrote:
>>
>>> On Thu, Mar 24, 2011 at 18:51, Jim Meyering <jim@meyering.net> wrote:
>>>> Janne Blomqvist wrote:
>>>>> On Tue, Mar 8, 2011 at 19:53, Jim Meyering <jim@meyering.net> wrote:
>>>>>> Relative to v2, I've added libgo/ to the list of exempt
>>>>>> directories and added
>>>>>> this recently discussed gfc_free patch, at the request of Tobias Burnus.
>>>>>> Also, I corrected an error in fortran's ChangeLog and removed all
>>>>>> whitespace changes from all ChangeLog files.
>>>>>
>>>>> The libgfortran changes are Ok for 4.7.
>>>>>
>>>>> For the gfortran frontend (gcc/fortran/*) I'd prefer if you'd
>>>>>
>>>>> - Replace all calls to "gfc_free (x)" with "free (x)".
>>>>> - Remove the gfc_free() function and prototype.
>>>>> - Remove the free() macro which currently prevents calling free() directly.
>>>>
>>>> Following up, I've refreshed the series but hit a minor snag
>>>> while converting new uses of gfc_free, removing new tests-before-free
>>>> and merging/reordering changes.
>>>>
>>>> Applying this fix first makes my problem go away:
>>>
>>> [snip]
>>>
>>> So, what's the plan here? Do you plan to get a GCC account, do you
>>> already have one, or what? Now that 4.7 is open for development, it's
>>> perhaps the right time to poke the maintainers to get this patch in.
>>>
>>> If you don't have a GCC account, as one of the Fortran maintainers I
>>> can commit the Fortran and libgfortran parts, but someone else will
>>> have to do the rest (were they ever approved, BTW?) as I have only
>>> write after approval privileges for the rest of GCC.
>>
>> Can someone add me to the gcc group?  That would help.
>> I already have ssh access to sourceware.org.
>
> According to http://gcc.gnu.org/svnwrite.html , you should send an
> email to overseers(at)gcc.gnu.org . It says that you need a sponsor
> and "The steering committee or a well-established GCC maintainer
> (including reviewers) can approve for write access any person with GNU
> copyright assignment papers in place and known to submit good
> patches.". I'm not sure if I'm considered to be well-established
> enough, so could someone help Jim out here, please?
>
> Or, if nobody pipes up within a few days, just mention my name as your
> sponsor and we'll see if the overseers consider me well-established or
> not.. ;)

Thanks.
FYI, I have an "ANY" assignment on file, so no need to wait for paperwork.
Tom Tromey - April 15, 2011, 1:20 p.m.
>>>>> "Janne" == Janne Blomqvist <blomqvist.janne@gmail.com> writes:

Jim> Can someone add me to the gcc group?  That would help.
Jim> I already have ssh access to sourceware.org.

Janne> I'm not sure if I'm considered to be well-established
Janne> enough, so could someone help Jim out here, please?

I added Jim to the gcc group.

Tom
Jim Meyering - April 15, 2011, 5:52 p.m.
> I added Jim to the gcc group.

Thanks, Tom.

Patch

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 4e0a792..c532986 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,8 @@ 
+2011-03-24  Jim Meyering  <meyering@redhat.com>
+
+	* trans-openmp.c (gfc_trans_omp_reduction_list): Do not guard
+	use of gfc_free; it can handle a NULL argument.
+
 2010-03-21  Thomas Koenig  <tkoenig@gcc.gnu.org>

 	PR fortran/22572
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 53eb999..77ed3bb 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -714,8 +714,7 @@  gfc_trans_omp_array_reduction (tree c, gfc_symbol *sym, locus where)
   gfc_free (symtree1);
   gfc_free (symtree2);
   gfc_free (symtree3);
-  if (symtree4)
-    gfc_free (symtree4);
+  gfc_free (symtree4);
   gfc_free_array_spec (outer_sym.as);
 }