On 11/18/14 0:38, Joseph Myers wrote:
> On Sat, 15 Nov 2014, Chen Gang wrote:
> 
>> Also in c_common_read_pch(), when failure occurs, also need be sure the
>> 'fd' is not '-1' for the next close operation.
> 
> Please clarify how c_common_read_pch gets called with fd == -1.  
> Certainly checking after an error is too late; we shouldn't call fdopen in 
> that case at all, and I think something further up the call chain should 
> have avoided calling c_common_read_pch with fd == -1 at all (the question 
> is just exactly what point on the call chain is missing such a check and 
> needs it added).
> 

According to current source code, the author wants 'fd' should never be
'-1' in c_common_read_pch(). "grep -rn read_pch *" in root directory,
then "grep -rn _cpp_stack_file *", can know it is used in 3 areas:

 - c_common_pch_pragma() in "gcc/c-family/c-pch.c", it has already
   checked 'fd' before call c_common_read_pch()

 - _cpp_stack_include() in "libcpp/files.c", before _cpp_stack_file(),
   has called and checked _cpp_find_file().

 - cpp_read_main_file() in "libcpp/init.c", before _cpp_stack_file(),
   has called and checked _cpp_find_file().

In c_common_read_pch(), even if 'fd' is '-1', we should check it firstly
before call fdopen(), instead of check '-1' in failure code block.

But for me, it contents another 2 related issues:

 - _cpp_find_file() always return a valid pointer, so related check code
   "file == NULL" is no use for _cpp_find_file() in _cpp_stack_include().

 - According to its comments, _cpp_find_file() can not be sure of
   'file->fd' must not be '-1', even when "file->err_no == 0", we need
   reopen it if necessary.

Thanks.
-- 
Chen Gang

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

Reply via email to