Re: [PATCH] Use "|" instead of "+" when combine the _IO_LINE_BUF and _IO_UNBUFFERED flags

[Feng Gao <gfree dot wind at gmail dot com>]

Hi Siddhesh,

Thanks your response.
The attachment is the latest change according to your suggestion
including the ChangeLog change.

About the tests, I did the following cases:
1. Write test codes to check if the (_IO_LINE_BUF|_IO_UNBUFFERED)
equals  (_IO_LINE_BUF+_IO_UNBUFFERED);
2. Update the glibc to check if it works like before.

On Tue, Jul 7, 2015 at 3:31 PM, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:
> Please describe the testing you have done to verify that your patch is
> correct.  In general for such monotonous changes, it is sufficient to
> verify that there is no change in the generated code or any change is
> expected and contained.  Also, run the testsuite to verify that no
> additional errors are introduced.
>
> Add a space between the operator; I know this was wrong before, but it
> should be corrected with this patch.  That is,
>
>     _IO_LINE_BUF|_IO_UNBUFFERED
>
> should be
>
>     _IO_LINE_BUF | _IO_UNBUFFERED
>
> Your patch also needs a ChangeLog entry.  The patch is probably
> trivial enough that it does not need a copyright assignment.
>
> Please post the patch with the suggested change, a ChangeLog entry and
> a description of how you tested the patch.
>
> Thanks,
> Siddhesh
>
> On Tue, Jun 30, 2015 at 07:23:57AM +0800, Feng Gao wrote:
>> Hi all,
>>
>> Both of "_IO_UNBUFFERED" and "_IO_LINE_BUF"  are the bit flags, but I
>> find there are some codes looks like "_IO_LINE_BUF+_IO_UNBUFFERED",
>> while some codes are "_IO_LINE_BUF|_IO_UNBUFFERED".
>>
>> I think the former is not good, even though the final result is same.
>>
>> The attachment is my patch .
>>
>> Thanks
>> Feng
>
>> diff --git a/libio/fileops.c b/libio/fileops.c
>> index 7c7fef1..93d0d94 100644
>> --- a/libio/fileops.c
>> +++ b/libio/fileops.c
>> @@ -541,7 +541,7 @@ new_do_write (fp, data, to_do)
>>    _IO_setg (fp, fp->_IO_buf_base, fp->_IO_buf_base, fp->_IO_buf_base);
>>    fp->_IO_write_base = fp->_IO_write_ptr = fp->_IO_buf_base;
>>    fp->_IO_write_end = (fp->_mode <= 0
>> -                    && (fp->_flags & (_IO_LINE_BUF+_IO_UNBUFFERED))
>> +                    && (fp->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
>>                      ? fp->_IO_buf_base : fp->_IO_buf_end);
>>    return count;
>>  }
>> @@ -881,7 +881,7 @@ _IO_new_file_overflow (f, ch)
>>        f->_IO_read_base = f->_IO_read_ptr = f->_IO_read_end;
>>
>>        f->_flags |= _IO_CURRENTLY_PUTTING;
>> -      if (f->_mode <= 0 && f->_flags & (_IO_LINE_BUF+_IO_UNBUFFERED))
>> +      if (f->_mode <= 0 && f->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
>>       f->_IO_write_end = f->_IO_write_ptr;
>>      }
>>    if (ch == EOF)
>> diff --git a/libio/oldfileops.c b/libio/oldfileops.c
>> index c68ca6a..343875a 100644
>> --- a/libio/oldfileops.c
>> +++ b/libio/oldfileops.c
>> @@ -313,7 +313,7 @@ old_do_write (fp, data, to_do)
>>      fp->_cur_column = _IO_adjust_column (fp->_cur_column - 1, data, count) + 1;
>>    _IO_setg (fp, fp->_IO_buf_base, fp->_IO_buf_base, fp->_IO_buf_base);
>>    fp->_IO_write_base = fp->_IO_write_ptr = fp->_IO_buf_base;
>> -  fp->_IO_write_end = ((fp->_flags & (_IO_LINE_BUF+_IO_UNBUFFERED))
>> +  fp->_IO_write_end = ((fp->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
>>                      ? fp->_IO_buf_base : fp->_IO_buf_end);
>>    return count;
>>  }
>> @@ -418,7 +418,7 @@ _IO_old_file_overflow (f, ch)
>>        f->_IO_write_end = f->_IO_buf_end;
>>        f->_IO_read_base = f->_IO_read_ptr = f->_IO_read_end;
>>
>> -      if (f->_flags & (_IO_LINE_BUF+_IO_UNBUFFERED))
>> +      if (f->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
>>       f->_IO_write_end = f->_IO_write_ptr;
>>        f->_flags |= _IO_CURRENTLY_PUTTING;
>>      }
>> diff --git a/libio/wfileops.c b/libio/wfileops.c
>> index 3f628bf..14332ac 100644
>> --- a/libio/wfileops.c
>> +++ b/libio/wfileops.c
>> @@ -106,7 +106,7 @@ _IO_wdo_write (fp, data, to_do)
>>            fp->_wide_data->_IO_buf_base);
>>    fp->_wide_data->_IO_write_base = fp->_wide_data->_IO_write_ptr
>>      = fp->_wide_data->_IO_buf_base;
>> -  fp->_wide_data->_IO_write_end = ((fp->_flags & (_IO_LINE_BUF+_IO_UNBUFFERED))
>> +  fp->_wide_data->_IO_write_end = ((fp->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
>>                                  ? fp->_wide_data->_IO_buf_base
>>                                  : fp->_wide_data->_IO_buf_end);
>>
>> @@ -465,7 +465,7 @@ _IO_wfile_overflow (f, wch)
>>        f->_IO_read_base = f->_IO_read_ptr = f->_IO_read_end;
>>
>>        f->_flags |= _IO_CURRENTLY_PUTTING;
>> -      if (f->_flags & (_IO_LINE_BUF+_IO_UNBUFFERED))
>> +      if (f->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
>>       f->_wide_data->_IO_write_end = f->_wide_data->_IO_write_ptr;
>>      }
>>    if (wch == WEOF)
>

Attachment: file_flags.diff
Description: Text document