This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.



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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]