This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: "Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>
- Cc: libc-alpha at sourceware dot org
- Date: Thu, 15 Nov 2018 09:26:25 -0800
- Subject: Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.
- References: <20181029121650.24544-1-gabriel@inconstante.eti.br> <20181029121650.24544-8-gabriel@inconstante.eti.br> <24a6e442-e6a2-6127-8e4a-5bbc906cb83e@linaro.org> <20181115130834.7aba7e50@tereshkova.br.ibm.com>
On 15/11/2018 07:08, Gabriel F. T. Gomes wrote:
> On Thu, 08 Nov 2018, Adhemerval Zanella wrote:
>
>> On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:
>>
>>> Signed-off-by: Zack Weinberg <zackw@panix.com>
>>> Signed-off-by: Gabriel F. T. Gomes <gabriel@inconstante.eti.br>
>>
>> We don't use DCO, but copyright assignments.
>
> Ack, fixed.
>
>>> --- /dev/null
>>> +++ b/debug/vobprintf_chk.c
>>> @@ -0,0 +1,32 @@
>>> +/* Print output of stream to given obstack.
>>> + Copyright (C) 1996-2018 Free Software Foundation, Inc.
>>> + This file is part of the GNU C Library.
>>> + Contributed by Ulrich Drepper <drepper@cygnus.com>, 1996.
>>
>> I think it is better to just set Copyright to 2018 without 'Contributed by'
>> (this implementation is quite different than the original one).
>
> Ack, fixed.
>
>>> -___sprintf_chk (char *s, int flags, size_t slen, const char *format, ...)
>>> +___sprintf_chk (char *s, int flag, size_t slen, const char *format, ...)
>>> [...]
>>> + if (slen == 0)
>>> + __chk_fail ();
>>
>> Maybe a __glibc_unlikely here?
>
> Looks right to me, added. Please see comments below.
>
>>> __nldbl___vsprintf_chk (char *string, int flag, size_t slen, const char *fmt,
>>> va_list ap)
>>> {
>>> + if (slen == 0)
>>> + __chk_fail ();
>
> I added __glibc_unlikely here, too.
>
>>> int
>>> -___vsprintf_chk (char *s, int flags, size_t slen, const char *format,
>>> - va_list args)
>>> +___vsprintf_chk (char *s, int flag, size_t slen, const char *format,
>>> + va_list ap)
>>> [...]
>>> if (slen == 0)
>>> __chk_fail ();
>
> This is an unchanged hunk (brought in by diff context), but may I also add
> __glibc_unlikely to it along with the other changes?
>
> These are the only ocurrences of slen == 0 (__nldbl___sprintf_chk calls
> __nldbl___vsprintf_chk, so it doesn't check slen on its own).
>
As Florian has pointed out I also think this is not really necessary.
So I am ok with current changes.