Re: [PATCH 1/* v3] Generic string function optimization: Add skeleton

[Joseph Myers <joseph at codesourcery dot com>]

On Fri, 29 May 2015, OndÅej BÃlka wrote:

> > If, instead, the code does
> > 
> > #if MACRO
> > 
> > and there's a sysdeps/generic header that defines MACRO one way, if an 
> > architecture overrides that header with one that misspells the name, a 
> > -Wundef warning will be immediately visible (though we still need to fix 
> > the -Wundef warnings in the testsuite and remove the -Wno-error=undef).
> > 
> Joseph, read previous mail before writing. Your suggestion is pointless.

Which previous mail?  As far as I can tell, your last patch posting adding 
common.h is <https://sourceware.org/ml/libc-alpha/2015-05/msg00751.html>, 
which has a range of within-function #ifdefs (some completely untested, 
e.g. "#  ifdef NEED BITWISE", and all completely undocumented).  Anyway, 
when we have conventions in glibc (such as preferring #if to #ifdef) you 
should follow them unless there is a strong justification for doing 
otherwise (presented in every patch submission).  Likewise, even early 
patches should follow normal glibc formatting conventions.

Throwing out a series of untested, undocumented patches is not a helpful 
way of proposing changes to glibc.  I strongly recommend, in any 
complicated case such as this, that:

(a) each patch submission is self-contained - has the full self-contained 
write-up of the patch itself with the rationale for the patch and all the 
choices made, that would go in the commit message, followed by the 
description of changes from the previous version, rather than requiring a 
trail of previous messages to be followed to get the full rationale; and

(b) the documentation is more important than the code (write first for 
humans to read, only then for computers to execute); documenting the 
interfaces (such as FAST_CLZ and NEED_BITWISE) should be a very early step 
before any patches are sent to the list, not an afterthought, and the same 
applies to each internal function and macro in the code, even those that 
are not interfaces for architectures to reimplement.

> For a code
> 
> #ifndef CUSTOM_FOO
> int foo()
> {
> }
> #endif
> 
> If architecture does
> #define CUTSOM_FOO
> int foo()
> {
> }
> 
> Then it gets error with redefinition of foo.

Or you could avoid making readers think about whether the #ifndef is OK in 
a particular case by simply following normal glibc practices and have a 
sysdeps/generic/string-foo.h header that has a default definition with a 
careful comment explaining the semantics, and then architectures can have 
their own version to replace it as needed; no macros needed at all.

-- 
Joseph S. Myers
joseph@codesourcery.com