eglibc option groups fixup

This is a first pass at addressing 6796, 6797, and 6809. For the latter two, I've just adopted the suggested fixes, for the former I've dropped an #if that I don't *think* we need. This might produce bogus results with the locale option group disabled, but it'll work in the normal case and reduce the number of -Wundef warnings and build problems.

The following changes since commit dd93b3a950d063f4e3020362c636006f51fb5782:

image_types.bbclass: whitespace and reorder (2014-11-06 16:41:38 +0000)

are available in the git repository at:
git://git.yoctoproject.org/poky-contrib seebs/glibcoptions
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=seebs/glibcoptions

Peter Seebach (1):
eglibc-use-option-groups.patch: Several fixups

 .../glibc/glibc/eglibc-use-option-groups.patch     |   40 +------------------
 1 files changed, 3 insertions(+), 37 deletions(-)

This incorporates suggested changes for issues 6797 and 6809 from bugzilla, and a proposed change for 6796. For 6796, the issue is that you can't just include <gnu/option-groups.h> in libc-symbols, so we may not always have the option group defines available. The workaround is to just leave the HAVE_MBSTATE_T definition visible; the obvious relevant usage is already also conditionalized on the option group locale define by this patch. This might not fix all the cases if the locale group isn't enabled, but it eliminates a pretty large raft of -Wundef warnings.

The reason HAVE_MBSTATE_T wasn't conditionalized in 2.19 is that it didn't exist in 2.19.

---
 .../glibc/glibc/eglibc-use-option-groups.patch     |   40 +------------------
 1 files changed, 3 insertions(+), 37 deletions(-)

index 40c7832..e4a0502 100644
@@ -2434,24 +2434,8 @@ Index: git/localedata/Makefile
  
  ifeq ($(run-built-tests),yes)
  # We have to generate locales
-@@ -143,9 +161,13 @@
- $(addprefix $(objpfx),$(CTYPE_FILES)): %: \
-   gen-locale.sh $(common-objpfx)locale/localedef Makefile \
-   $(addprefix charmaps/,$(CHARMAPS)) $(addprefix locales/,$(LOCALE_SRCS))
--	@$(SHELL) gen-locale.sh $(common-objpfx) \
--		  '$(built-program-cmd-before-env)' '$(run-program-env)' \
--		  '$(built-program-cmd-after-env)' $@; \
-+	@$(SHELL) gen-locale.sh $(common-objpfx)	\
-+		 '$(if $(cross-localedef), 		\
-+		       $(cross-localedef),		\
-+		       $(built-program-cmd-before-env)  \
-+		       $(run-program-env)		\
-+		       $(built-program-cmd-after-env))' \
-+		       $@; \
- 	$(evaluate-test)
- 
  $(addsuffix .out,$(addprefix $(objpfx),$(tests))): %: \
-@@ -213,6 +235,11 @@
+@@ -213,6 +231,11 @@
  
  include SUPPORTED
  

@@ -5065,8 +5049,8 @@ Index: git/posix/Makefile
  		 $(objpfx)bug-glob2-mem.out $(objpfx)tst-vfork3-mem.out \
  		 $(objpfx)tst-fnmatch-mem.out $(objpfx)bug-regex36-mem.out
 +ifeq (y,$(OPTION_POSIX_REGEXP_GLIBC))
-+tests-special += $(objpfx)bug-regex14-mem $(objpfx)tst-rxspencer-no-utf8-mem \
-+  		 $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem
++tests-special += $(objpfx)bug-regex14-mem.out $(objpfx)tst-rxspencer-no-utf8-mem.out \
++  		 $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem.out
 +endif
 +
  xtests-special += $(objpfx)bug-ga2-mem.out

@@ -16458,24 +16442,6 @@ Index: git/sysdeps/nptl/small-macros-fns.c
 +libc_hidden_def (__libc_lock_unlock_recursive_fn);
 +
 +#endif /*defined _LIBC && (!defined NOT_IN_libc || defined IS_IN_libpthread)*/
-Index: git/include/libc-symbols.h
-===================================================================
---- git.orig/include/libc-symbols.h	2014-08-29 20:00:47.144070587 -0700
-+++ git/include/libc-symbols.h	2014-08-29 20:01:15.248070587 -0700
-@@ -60,8 +60,11 @@
- /* Define these macros for the benefit of portable GNU code that wants to check
-    them.  Of course, STDC_HEADERS is never false when building libc!  */
- #define STDC_HEADERS	1
--#define HAVE_MBSTATE_T	1
--#define HAVE_MBSRTOWCS	1
-+
-+#if __OPTION_EGLIBC_LOCALE_CODE
-+# define HAVE_MBSTATE_T	1
-+# define HAVE_MBSRTOWCS	1
-+#endif
- #define HAVE_LIBINTL_H	1
- #define HAVE_WCTYPE_H	1
- #define HAVE_ISWCTYPE	1
 Index: git/crypt/crypt_common.c
 ===================================================================
 --- /dev/null	1970-01-01 00:00:00.000000000 +0000

1.7.1

Whilst I haven't 100% confirmed it,
https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps/BuildImages/logs/stdio
looks like it may well be as a result of this change (which was included in the master-next build in question)...

Cheers,

Richard

Hmm. I am *pretty* sure I was successfully building things with this patch. But it does look suspiciously similar.

-s

Listen, get this.  Nobody with a good compiler needs to be justified.

It is 100% confirmed now, with the patch reverted there was a green build.

Cheers,

Richard

Drat. I'll study this more and try to figure out what I missed.

-s

Listen, get this.  Nobody with a good compiler needs to be justified.

https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps/...

Peter, I noticed you have another patch that fixes this issue:

@@ -16469,7 +16452,7 @@ Index: git/include/libc-symbols.h -#define HAVE_MBSTATE_T 1
-#define HAVE_MBSRTOWCS 1
+
-+#if __OPTION_EGLIBC_LOCALE_CODE
++#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE +# define HAVE_MBSTATE_T 1
+# define HAVE_MBSRTOWCS 1
+#endif

This will fix the the issue, but (please correct me if I'm wrong) the way I see it, since__OPTION_EGLIBC_LOCALE_CODE is defined in gnu/option-groups.h, which is NOT included libc-symbols.h and hence

	#if defined(__OPTION_EGLIBC_LOCALE_CODE) 

will always evaluate as false and HAVE_MBSTATE_T will be never defined.

I am not sure why the HAVE_MBSTATE_T needs to be conditioned at all, my understanding is
it is defined if the file <wchar.h> contains type definition "mbstate_t". This typedef either exists or not and saying it not exists when in fact it does might be confusing (as it already is for me) and I don't see any relation with __OPTION_EGLIBC_LOCALE_CODE.

The only place in glibc I could find that uses HAVE_MBSTATE_T is in fnmatch.c:

#if defined _LIBC

# include <gnu/option-groups.h>
#endif

# if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS) || (_LIBC && __OPTION_EGLIBC_LOCALE_CODE)
# define HANDLE_MULTIBYTE 1
# endif

To me it seems it is actually HANDLE_MULTIBYTE that we want conditioned with a new patch, resulting in something like:

#if defined _LIBC

# include <gnu/option-groups.h>
#endif

# if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS && _LIBC && __OPTION_EGLIBC_LOCALE_CODE)
# define HANDLE_MULTIBYTE 1
# endif

Incidentally, I tried a core-image-minimal build with unconditioned HAVE_MBSTATE_T and encountered no problems, just like you did originally. This would imply poky-tiny problem is a probably a different problem.

One last unrelated thing: I believe in the patch you missed one ".out" extension,
I think it should be:

...
++ $(objpfx)tst-pcre-mem.out $(objpfx)tst-boost-mem.out
...

instead of

...
++ $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem.out
...

https://autobuilder.yoctoproject.org/main/builders/poky-tiny/builds/102/steps
/BuildImages/logs/stdio

Peter, I noticed you have another patch that fixes this issue:

@@ -16469,7 +16452,7 @@ Index: git/include/libc-symbols.h -#define HAVE_MBSTATE_T 1
-#define HAVE_MBSRTOWCS 1
+
-+#if __OPTION_EGLIBC_LOCALE_CODE
++#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE +# define HAVE_MBSTATE_T 1
+# define HAVE_MBSRTOWCS 1
+#endif

This will fix the the issue, but (please correct me if I'm wrong) the way I see it, since__OPTION_EGLIBC_LOCALE_CODE is defined in gnu/option-groups.h, which is NOT included libc-symbols.h and hence

	#if defined(__OPTION_EGLIBC_LOCALE_CODE) 

will always evaluate as false and HAVE_MBSTATE_T will be never defined.

I am not sure why the HAVE_MBSTATE_T needs to be conditioned at all, my understanding is
it is defined if the file <wchar.h> contains type definition "mbstate_t". This typedef either exists or not and saying it not exists when in fact it does might be confusing (as it already is for me) and I don't see any relation with __OPTION_EGLIBC_LOCALE_CODE.

The only place in glibc I could find that uses HAVE_MBSTATE_T is in fnmatch.c:

#if defined _LIBC

# include <gnu/option-groups.h>
#endif

# if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS) || (_LIBC && __OPTION_EGLIBC_LOCALE_CODE)
# define HANDLE_MULTIBYTE 1
# endif

To me it seems it is actually HANDLE_MULTIBYTE that we want conditioned with a new patch, resulting in something like:

#if defined _LIBC

# include <gnu/option-groups.h>
#endif

#if (HAVE_MBSTATE_T && HAVE_MBSRTOWCS)
# if _LIBC
# if __OPTION_EGLIBC_LOCALE_CODE
# define HANDLE_MULTIBYTE 1
# endif
# else
# define HANDLE_MULTIBYTE 1
# endif
#endif

Incidentally, I tried a core-image-minimal build with unconditioned HAVE_MBSTATE_T and encountered no problems, just like you did originally. This would imply poky-tiny problem is a probably a different problem. (The suggested patch in fnmatch.c could possibly fix it as well)

One last unrelated thing: I believe in the patch you missed one ".out" extension,
I think it should be:

...
++ $(objpfx)tst-pcre-mem.out $(objpfx)tst-boost-mem.out
...

instead of

...
++ $(objpfx)tst-pcre-mem $(objpfx)tst-boost-mem.out
...

If it's not conditional, the build fails when the feature's not included, because one of the test programs attempts to use it. It may be that something else ought to be conditional to make that work. So it might be that your suggested change to fnmatch would fix poky-tiny as well.

And you're right about the .out.

-s

Listen, get this.  Nobody with a good compiler needs to be justified.

Hello Peter,
my concern is that if the file libc-symbols.h is patched like this:

#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE

# define HAVE_MBSTATE_T    1
# define HAVE_MBSRTOWCS    1
#endif

then it will be ALWAYS evaluated as:

#if 0

# define HAVE_MBSTATE_T    1
# define HAVE_MBSRTOWCS    1
#endif

because

#if defined __OPTION_EGLIBC_LOCALE_CODE

will always evaluate as 0.

To test this hypothesis, I actually build core-image-minimal with an explicit #error:

#if defined(__OPTION_EGLIBC_LOCALE_CODE) && __OPTION_EGLIBC_LOCALE_CODE

# error "I was wrong in my assumptions!!!"
# define HAVE_MBSTATE_T    1
# define HAVE_MBSRTOWCS    1
#endif

The image built fine, (Though I did not build/ran the glibc testsuite yet, nor did I try to build tiny-poky).

I believe the purpose of this patching is simply to unconditionally prevent suppport for mutibyte character handling if __OPTION_EGLIBC_LOCALE_CODE=0
If this is indeed the case, then the files posix/fnmatch.c and (possibly posix/fnmatch_loop.c may need revised patching. ).

Juro