This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Ping: [RFC][PATCH] Initial support for C11 Annex K Bounds checking functions
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Ulrich Bayer <ubayer at sba-research dot org>
- Cc: <libc-alpha at sourceware dot org>
- Date: Wed, 15 May 2013 16:11:26 +0000
- Subject: Re: Ping: [RFC][PATCH] Initial support for C11 Annex K Bounds checking functions
- References: <5102DBFD dot 4060103 at sba-research dot org> <513DE7A1 dot 8080501 at sba-research dot org>
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