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: [RFC][PATCH v2] Initial support for C11 Annex K Bounds checking functions


Thank you for your quick comments!

On 05.06.2013 19:03, Joseph S. Myers wrote:
>> diff --git a/lib_s/abort_handler_s.c b/lib_s/abort_handler_s.c
>> new file mode 100644
>> index 0000000..75b7fef
>> --- /dev/null
>> +++ b/lib_s/abort_handler_s.c
>> @@ -0,0 +1,38 @@
>> +/* Copyright (C) 2013 Free Software Foundation, Inc.
> 
> The first line of each new file should be a descriptive comment.  The 
> copyright notice comes after that description of the file.  (Comment 
> applies to other files as well.)
> 
>> +void
>> +abort_handler_s (const char *restrict msg, void *restrict ptr, errno_t error)
> 
> Please include a comment on each function definition with a brief 
> description of the function semantics and those of the parameters, as 
> described in the GNU Coding Standards.  (Likewise for other functions.)
Ok.

> 
>> +/* The strcpy_s function copies the string s2 to s1.
>> +   Unlike the traditional strcpy, the size of the destination buffer s1
>> +   has to be specified. The function makes sure that it never
>> +   writes outside the specified bounds.
>> +*/
>> +extern errno_t strcpy_s (char * __restrict__ s1, rsize_t s1max,
>> +			 const char * __restrict__ s2)
>> +  __THROW __nonnull ((1,3));
> 
> I don't think it's appropriate to use __THROW on any function with runtime 
> constraints; it's reasonable for someone to set a runtime constraint 
> handler that throws C++ exceptions.

You are right. I have removed all __THROWS in all functions declarations except 
for the functions 
set_constraint_handler_s and 
ignore_handler_s 
because these two do not cause any runtime-constraint violations.
 
> Likewise, it's inappropriate to use the nonnull attribute on arguments 
> where the semantics if those arguments are NULL is specified (as a runtime 
> constraint violation), because GCC may optimize based on the argument not 
> being NULL - the attribute can be used only when a NULL argument means 
> undefined behavior.

I tend to agree. It's a pity we loose GCC's warnings about null pointer 
arguments though. I don't like the fact that a program becomes more unsecure 
by the fact that is uses the Annex K functions. Although the _s functions 
gracefully handle the case when a NULL pointer is invoked, the user should not 
call the function with NULL as it results in a runtime-constraint violation. 
A compiler warning that something is NULL is always preferable to a runtime 
constraint (which might be an abort). Is there any other way so that GCC could
warn about NULL but suppress optimization?



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