[PATCH] gcc/toplev.c: Avoid to close 'asm_out_file' when it is 'stdout'

[Chen Gang]

On 07/26/2014 05:12 AM, Jeff Law wrote:
> On 07/23/14 16:17, Chen Gang wrote:
>> On 07/23/2014 11:44 AM, Jeff Law wrote:
>>> On 07/21/14 09:47, Chen Gang wrote:
>>>> 'asm_out_file' may be 'stdout', so need check this case before close
>>>> it.
>>>> Or 'stdout' may be closed -- since need not open 'stdout', either need
>>>> not close it.
>>>>
>>>> ChangLog:
>>>>
>>>>     * topleve.c (finalize): Avoid to close 'asm_out_file' when it is
>>>>     'stdout'.
>>> What exactly is the problem with closing stdout at this point?  In
>>> general, you need to state the problem you're trying to fix with your
>>> patch.
>>>
>>
>> Excuse me, I only find it by reading source code, so for me, I didn't
>> meet the real problem for it, so at least, this patch is not urgent (
>> although I am not sure whether it is still valuable or not).
> OK.  I suspected it might be the case that you just saw something odd
> and sent a patch to change it.
> 
> But that was just a suspicion -- there well could have been some path
> you found where GCC wanted to write to stdout after the point where we
> closed the output file.  That would clearly be a bug and warrant fixing
> immediately.
> 

In source root directory:
  "grep -rn asm_out_file * | grep stdout"
  "grep -rn asm_out_file * | grep fclose"
  "grep -rn asm_out_file * | grep NULL".

We can know about stdout may be used (e.g. the related parameter is '-')
in init_asm_output() in "gcc/toplev.c". And only one place to close it
in finalize() in "gcc/toplev.c".

But really, it is only by reading source code, no any real test for it,
so yeah, we can say it is only a suspicion.

> Either way it's important you tell us why you're making a change in a
> way which allows us to evaluate the importance of the change.  Otherwise
> we have to guess and we could easily guess wrong.
> 
> In this specific instance, I don't really see any value in avoiding the
> close of stdout.
> 

I don't see either, so at least, it is not urgent.

But if it was really a bug, for me, we have to fix it, when we have time.

>>
>> At present, I am a newbie, and use 2 ways to learn gcc and binutils.
>>
>>   - Cross compile the cross compiler with '-W' for linux kernel.
>>     (If find issues, I shall try to fix them with related members).
>>
>>   - Reading source code of gcc and binutils, if find some where can be
>>     improved, and try to send patch for it.
> And these are excellent ways to get started.  Another would be to build
> GCC with Clang/LLVM and see what warnings that generates and try to fix
> them.  Perusing the bug database (gcc.gnu.org/bugzilla) can sometimes
> result in identifying issues you can resolve.
> 

Thank you for your ideas and suggestions, Clang/LLVM and bugzilla are
really two important and valuable places.

But excuse me, I have no enough time resources on it. I have started
Linux kernel and qemu/kvm/xen in my free time, and now, I am starting
gcc/binutils in my free time, too.

 - "cross compiling the cross-compiler for linux kernel, and let each
   architectures in kernel pass allmodconfig" is an efficient way to me
   for both learning gcc/binutils and linux kernel.

 - Reading source code are always necessary in any cases (it is my main
   learning way for qemu/kvm/xen).

Clang/LLVM and bugzilla are really very important, so I shall try them
in the future (I guess, may be next year -- 2015).

>> By the way, is there a trivial patch mailing list of gcc? I guess most
>> of my patches belong to trivial.
> No, all patches go to gcc-patches, even trivial ones.
> 

OK, thanks. And I shall continue in gcc-patches.

And excuse me, next, most of my trivial patches maybe bother many other
members in gcc-patches mailing list.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed