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


On Mon, 11 Mar 2013, Ulrich Bayer wrote:

> Ping.
> I'd welcome your feedback.

At the time of the ping your copyright assignment hadn't yet been 
processed by the FSF, but I see it's now listed in copyright.list (with a 
date of 26 March).  I suggest reposting a patch against current master and 
hopefully we can at least get a discussion going on it, whether or not it 
actually gets in for 2.18.

Some observations on the patch as at 
<http://sourceware.org/ml/libc-alpha/2013-03/msg00286.html>, mainly 
regarding coding style, to correct when posting an updated patch:

* You have "\ No newline at end of file" in Versions.def, make sure all 
text files end with a final newline (exactly one, otherwise the git commit 
hooks will complain).

* I don't think the check for identical definitions of 
__STDC_WANT_LIB_EXT1__ is worth probably breaking GCC's multiple include 
optimization for features.h; since full implementation of the standard 
check (that the definition is identical at each include of a relevant 
header, including repeated inclusions of the same header and including 
variations in token sequence or spelling) requires compiler support, I 
think that check is probably best implemented only with compiler support 
and without a fallback in features.h.

* Hopefully some version of my patch 
<http://sourceware.org/ml/libc-alpha/2013-05/msg00421.html> will go in and 
mean you no longer need dependencies no libc.so and libc_nonshared.a in 
lib_s/Makefile.

* In abort_handler_s.c, the function definition should have the return 
type on a separate line from the function name, so

void
abort_handler_s (...)

Likewise elsewhere (not specifically pointed out in other places).

* I think it's preferred to use comparisons e.g. "msg != NULL" or "msg == 
NULL" rather than just "if (msg)" or "if (!msg)".

* Full sentences in comments, starting with a capital letter, ending with 
"." and two spaces before the "*/" (e.g. "set it back to our default 
handler" in constraint_handler.c is badly formatted in this regard).  No 
"*" at start of continuation lines in comments (e.g. in 
ignore_handler_s.c).

* When an expression goes over multiple lines, operators go at the start 
rather than the end of a line (e.g. _REGIONS_OVERLAP in libc_s.h).

* Spaces after commas (e.g. "__FUNCTION__,__FILE__" missing a space in 
_CONSTRAINT_VIOLATION_IF).

* Spaces before '(' in calls to functions or function-like macros, and 
around '=', e.g. see "s2_len=STRNLEN_S(s2, s1max)" for bad formatting in 
strcpy_s.c (the space there in ")) )" is also spurious).

* In installed headers, use "__restrict" rather than "__restrict__" or 
plain "restrict".

* Is there a reason for the test macros in test-lib_s.h to use fprintf to 
stdout rather than just plain printf?

-- 
Joseph S. Myers
joseph@codesourcery.com


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