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,

Now I get the reason why the "make check" hang.
The "make check" need start after make. (I am a little curious why it
will hang if not make )

But the result is same whatever apply my change or not, it stops the
same step following:

gcc inl-tester.c -c -std=gnu99 -fgnu89-inline -fno-stack-protector -O2
-Wall -Werror -Wno-error=undef -Wundef -Wwrite-strings
-fmerge-all-constants -frounding-math -g -Wstrict-prototypes
-Wa,-mtune=i686 -fno-builtin    -U_FORTIFY_SOURCE   -I../include
-I/home/fgao/works/glibc-build/string  -I/home/fgao/works/glibc-build
-I../sysdeps/unix/sysv/linux/i386/i686  -I../sysdeps/i386/i686/nptl
-I../sysdeps/unix/sysv/linux/i386  -I../sysdeps/unix/sysv/linux/x86
-I../sysdeps/i386/nptl  -I../sysdeps/unix/sysv/linux/include
-I../sysdeps/unix/sysv/linux  -I../sysdeps/nptl  -I../sysdeps/pthread
-I../sysdeps/gnu  -I../sysdeps/unix/inet  -I../sysdeps/unix/sysv
-I../sysdeps/unix/i386  -I../sysdeps/unix  -I../sysdeps/posix
-I../sysdeps/i386/i686/fpu/multiarch  -I../sysdeps/i386/i686/fpu
-I../sysdeps/i386/i686/multiarch  -I../sysdeps/i386/i686
-I../sysdeps/i386/i486  -I../sysdeps/i386/fpu
-I../sysdeps/x86/fpu/include -I../sysdeps/x86/fpu  -I../sysdeps/i386
-I../sysdeps/x86  -I../sysdeps/wordsize-32
-I../sysdeps/ieee754/ldbl-96  -I../sysdeps/ieee754/dbl-64
-I../sysdeps/ieee754/flt-32  -I../sysdeps/ieee754
-I../sysdeps/generic  -I.. -I../libio -I.   -D_LIBC_REENTRANT -include
/home/fgao/works/glibc-build/libc-modules.h -DMODULE_NAME=nonlib
-include ../include/libc-symbols.h       -o
/home/fgao/works/glibc-build/string/inl-tester.o -MD -MP -MF
/home/fgao/works/glibc-build/string/inl-tester.o.dt -MT
/home/fgao/works/glibc-build/string/inl-tester.o
In file included from inl-tester.c:6:0:
tester.c: In function âtest_memcmpâ:
tester.c:1097:7: error: value computed is not used [-Werror=unused-value]
tester.c:1098:7: error: value computed is not used [-Werror=unused-value]
cc1: all warnings being treated as errors
make[2]: *** [/home/fgao/works/glibc-build/string/inl-tester.o] Error 1
make[2]: Leaving directory `/home/fgao/works/glibc/string'
make[1]: *** [string/tests] Error 2
make[1]: Leaving directory `/home/fgao/works/glibc'
make: *** [check] Error 2




On Wed, Jul 8, 2015 at 2:15 PM, Feng Gao <gfree.wind@gmail.com> wrote:
> Hi Siddhesh,
>
> Firstly, sorry about that I forget fixing the last space issue. Maybe
> I was very tried when it was very late in the night.
>
> Now the attachment "file_flags.diff" is the latest change which fix
> the issue you mentioned.
> The "test_flag.c" is the test codes to check the flag value.
> The "test_buf.c" is the test codes to test if the setbuf of libc works well.
>
> The following are the tests I did:
> 1. Run the test_flag.c on 32-bits and 64-bits platform, the output is "Equal".
>     It means (_IO_UNBUFFERED | _IO_LINE_BUF) has the same value of
> (_IO_UNBUFFERED + _IO_LINE_BUF);
> 2. After update the glibc, gdb the test_buf.c to check if the setbuf
> works well like before.
>     Because i am using the glibc-2.19 on my linux, so I apply my patch
> to glibc-2.19, build and replace the original one. ( I am afraid my
> computer become brick:))
>     Then gdb the binary of test_buf.c to check if the setbuf works
> well like before.
> Breakpoint 1, main () at test_buf.c:23
> 23      {
> (gdb) n
> 26              printf("Before no-buffer.");
> (gdb)
> 27              printf("Output now.\n");
> (gdb)
> Before no-buffer.Output now.
> 29              setbuf(stdout, NULL);
> (gdb)
> 30              printf("No buffer now.");
> (gdb)
> No buffer now.32                setvbuf(stdout, buffer, _IOLBF, sizeof(buffer));
> (gdb)
> 33              printf("Restore linebuf.");
> (gdb)
> 34              printf("Output now.\n");
> (gdb)
> Restore linebuf.Output now.
> 36              return 0;
>
> The output is the right behavior we expect.
>
> 3. I doesn't execute the "make check". Because it will hang without my
> patch. So I could not do "make check" with my patch.
> I don't know if there already is one bug in the current glibc codes.
>
> It stop the following step for about 2 hours, so I have to cancel it.
>
>         scripts/evaluate-test.sh c++-types-check $? false false >
> /home/fgao/works/my_git_codes/glibc-build/c++-types-check.test-result
> AWK='gawk' scripts/check-local-headers.sh \
>           "/usr/include" "/home/fgao/works/my_git_codes/glibc-build/"
>> /home/fgao/works/my_git_codes/glibc-build/check-local-headers.out; \
>         scripts/evaluate-test.sh check-local-headers $? false false >
> /home/fgao/works/my_git_codes/glibc-build/check-local-headers.test-result
>
> That's why I did not execute the "make check" with my change.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Wed, Jul 8, 2015 at 12:28 AM, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:
>> On Wed, Jul 08, 2015 at 12:02:38AM +0800, Feng Gao wrote:
>>> 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.
>>
>> Thanks, it looks like you missed fixing spacing in the last instance
>> (quoted below).  Also, run 'make check' before and after the patch to
>> make sure that there are no regressions due to this change.  Updating
>> glibc is a brave thing to do - if it breaks, your box is a brick that
>> only a rescue disk can get back :)
>>
>> Please post an updated patch and also let me know the results of your
>> test.
>>
>> Thanks,
>> Siddhesh
>>
>>> @@ -477,7 +477,7 @@ _IO_wfile_overflow (_IO_FILE *f, wint_t 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)
>>