| Message ID | 532B32B2.3020306@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
> Here is another warning fixing, powerpc specific. Following where it is > defined for other archs, I had added a new header for PowerPC. I am not > very found of adding another arch-specific header, however it seems the > way to avoid such define issues. We should not need a new header just for this. In this case, it seems appropriate for the generic bits/string.h to provide the common default. It looks like the most common value is 1, but I'm not entirely sure. We should take a poll of all the architectures. If there are only a few that want 0, then I'd say those few can add their own bits/string.h that defines it that way even if they have nothing else to define. (For example today sysdeps/m68k/m680x0/m68020/bits/string.h is nothing but setting it to 1. So if 1 is the common default, that file would be removed and arm, ia64, et al--whatever minority wants 0--would add files to set it to 0.) If in fact there is not a big disparity between the number that want it set to 0 but have no other bits/string.h content to provide and the number that want it set to 1 but have no other bits/string.h content to provide, then we should reconsider. In that sort of case it may well be most reasonable (i.e. best overall for avoiding typo-prone code balanced with avoiding duplicative code) to have string.h do #ifndef #define after including bits/string.h. Note that your patch is not just fixing the warning, it is changing the behavior because the missing macro was interpreted as 0 before. So I think we should treat this as two separate issues and possibly two separate changes: -Wundef cleanup; and changing the powerpc setting. Thanks, Roland
On Fri, 21 Mar 2014, Roland McGrath wrote: > We should not need a new header just for this. In this case, it seems > appropriate for the generic bits/string.h to provide the common default. > It looks like the most common value is 1, but I'm not entirely sure. We I think the common case, in terms of number of architectures, is probably 0. As I understand it, any architecture for which the GCC port defines STRICT_ALIGNMENT to 1, or SLOW_UNALIGNED_ACCESS to evaluate to 1 for the modes for which glibc can use unaligned accesses, should define _STRING_ARCH_unaligned to 0. (In cases where it depends on compiler options, a more careful examination is needed to work out what variants are actually relevant for glibc - it's quite possible that for some such architectures, all processors supported by glibc support unaligned operations for the relevant types.)
> I think the common case, in terms of number of architectures, is probably > 0. That is functionally consistent with the current code, where the generic file omits the macro. But let us measure rather than guess. Let someone (the arch maintainer or whoever gets to it) say for each arch what the proper value is, and let someone collate all those results.
diff --git a/sysdeps/powerpc/bits/string.h b/sysdeps/powerpc/bits/string.h new file mode 100644 index 0000000..eff3155 --- /dev/null +++ b/sysdeps/powerpc/bits/string.h @@ -0,0 +1,24 @@ +/* Optimized inlined string functions. PowerPC version. + Copyright (C) 2014 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef _STRING_H +# error "Never use <bits/string.h> directly; include <string.h> instead." +#endif + +#define _STRING_ARCH_unaligned 1