diff mbox

[ARM] PR driver/70132: Avoid double fclose in driver-arm.c

Message ID 56E2E510.10705@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov March 11, 2016, 3:32 p.m. UTC
Hi all,

As reported in the PR we can end up calling fclose twice on a file, causing an error.
This patch fixes that by reorganising the logic a bit to ensure we return after closing
the file the first time.

Bootstrapped and tested on arm-none-linux-gnueabihf

Ok for trunk?

Thanks,
Kyrill

2016-03-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR driver/70132
     * config/arm/driver-arm.c (host_detect_local_cpu): Set file pointer
     to NULL after closing file.

Comments

Bernd Schmidt March 14, 2016, 12:04 p.m. UTC | #1
On 03/11/2016 04:32 PM, Kyrill Tkachov wrote:
>      PR driver/70132
>      * config/arm/driver-arm.c (host_detect_local_cpu): Set file pointer
>      to NULL after closing file.

Doesn't match the patch. Either variant is fine but please use the right 
combination :)


Bernd
Ramana Radhakrishnan March 21, 2016, 10:41 a.m. UTC | #2
On Fri, Mar 11, 2016 at 3:32 PM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi all,
>
> As reported in the PR we can end up calling fclose twice on a file, causing
> an error.
> This patch fixes that by reorganising the logic a bit to ensure we return
> after closing
> the file the first time.
>
> Bootstrapped and tested on arm-none-linux-gnueabihf
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2016-03-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR driver/70132
>     * config/arm/driver-arm.c (host_detect_local_cpu): Set file pointer
>     to NULL after closing file.


OK with a fixed changelog.

Ramana
Kyrill Tkachov March 23, 2016, 10:19 a.m. UTC | #3
On 14/03/16 12:04, Bernd Schmidt wrote:
> On 03/11/2016 04:32 PM, Kyrill Tkachov wrote:
>>      PR driver/70132
>>      * config/arm/driver-arm.c (host_detect_local_cpu): Set file pointer
>>      to NULL after closing file.
>
> Doesn't match the patch. Either variant is fine but please use the right combination :)
>
>

Sorry, that ChangeLog was from a earlier revision of the patch.
The correct one is:

2016-03-23  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR driver/70132
     * config/arm/driver-arm.c (host_detect_local_cpu): Reorder exit logic
     to not call fclose twice on file.


Committed with r234419.

Thanks,
Kyrill

> Bernd
>
Kyrill Tkachov April 28, 2016, 9:24 a.m. UTC | #4
On 21/03/16 10:41, Ramana Radhakrishnan wrote:
> On Fri, Mar 11, 2016 at 3:32 PM, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> Hi all,
>>
>> As reported in the PR we can end up calling fclose twice on a file, causing
>> an error.
>> This patch fixes that by reorganising the logic a bit to ensure we return
>> after closing
>> the file the first time.
>>
>> Bootstrapped and tested on arm-none-linux-gnueabihf
>>
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2016-03-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR driver/70132
>>      * config/arm/driver-arm.c (host_detect_local_cpu): Set file pointer
>>      to NULL after closing file.
>
> OK with a fixed changelog.

Ok to backport this to the GCC 5 and 4.9 branches?
The tests are fine there for this patch.

Thank,
Kyrill

> Ramana
Ramana Radhakrishnan April 28, 2016, 9:53 a.m. UTC | #5
On Thu, Apr 28, 2016 at 10:24 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
>
> On 21/03/16 10:41, Ramana Radhakrishnan wrote:
>>
>> On Fri, Mar 11, 2016 at 3:32 PM, Kyrill Tkachov
>> <kyrylo.tkachov@foss.arm.com> wrote:
>>>
>>> Hi all,
>>>
>>> As reported in the PR we can end up calling fclose twice on a file,
>>> causing
>>> an error.
>>> This patch fixes that by reorganising the logic a bit to ensure we return
>>> after closing
>>> the file the first time.
>>>
>>> Bootstrapped and tested on arm-none-linux-gnueabihf
>>>
>>> Ok for trunk?
>>>
>>> Thanks,
>>> Kyrill
>>>
>>> 2016-03-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      PR driver/70132
>>>      * config/arm/driver-arm.c (host_detect_local_cpu): Set file pointer
>>>      to NULL after closing file.
>>
>>
>> OK with a fixed changelog.
>
>
> Ok to backport this to the GCC 5 and 4.9 branches?
> The tests are fine there for this patch.

OK.

Ramana

>
> Thank,
> Kyrill
>
>> Ramana
>
>
diff mbox

Patch

diff --git a/gcc/config/arm/driver-arm.c b/gcc/config/arm/driver-arm.c
index 466743b9d47f4144b6aade23e3f311405736ffa2..95dc9d53b6c179946d62f45b2b0d4a21960405b8 100644
--- a/gcc/config/arm/driver-arm.c
+++ b/gcc/config/arm/driver-arm.c
@@ -128,12 +128,11 @@  host_detect_local_cpu (int argc, const char **argv)
 	}
     }
 
-  fclose (f);
-
-  if (val == NULL)
-    goto not_found;
-
-  return concat ("-m", argv[0], "=", val, NULL);
+  if (val)
+    {
+      fclose (f);
+      return concat ("-m", argv[0], "=", val, NULL);
+     }
 
 not_found:
   {