GNU bug report logs - #17056
dfa.c patch for systems with no locale support

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17056 in the body.
You can then email your comments to 17056 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Aharon Robbins <arnold <at> skeeve.com>
To: bug-grep <at> gnu.org
Subject: dfa.c patch for systems with no locale support
Date: Fri, 21 Mar 2014 13:40:17 +0200

Hi.

This turned up in testing on DJGPP.  Thanks,

Arnold
--------------------------
diff --git a/dfa.c b/dfa.c
index 8771bbe..813c239 100644
--- a/dfa.c
+++ b/dfa.c
@@ -820,9 +820,13 @@ using_simple_locale (void)
       static int unibyte_c = -1;
       if (unibyte_c < 0)
         {
+#ifdef LC_ALL
           char *locale = setlocale (LC_ALL, NULL);
           unibyte_c = (locale && (STREQ (locale, "C")
                                   || STREQ (locale, "POSIX")));
+#else
+          unibyte_c = 1;
+#endif
         }
       return unibyte_c;
     }

Message #8 received at 17056 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Aharon Robbins <arnold <at> skeeve.com>, 17056 <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Fri, 21 Mar 2014 11:09:15 -0700

That's odd, as DJGPP has <locale.h> which is supposed to declare 
setlocale and define LC_ALL, according to 
<http://www.delorie.com/djgpp/doc/libc/libc_705.html>.  Could you 
explain what goes wrong?

Does the following patch fix the problem?  It should isolate the issue 
better.

diff --git a/src/dfa.c b/src/dfa.c
index 5e60cd5..92ac1b9 100644
--- a/src/dfa.c
+++ b/src/dfa.c
@@ -34,7 +34,11 @@
 #include <locale.h>
 #include <stdbool.h>

-/* Gawk doesn't use Gnulib, so don't assume static_assert is present.  */
+/* Gawk doesn't use Gnulib, so don't assume that setlocale and
+   static_assert are present.  */
+#ifndef LC_ALL
+# define setlocale(category, locale) NULL
+#endif
 #ifndef static_assert
 # define static_assert(cond, diagnostic) \
     extern int (*foo (void)) [!!sizeof (struct { int foo: (cond) ? 8 : 
-1; })]

Message #13 received at 17056 <at> debbugs.gnu.org (full text, mbox):

From: Aharon Robbins <arnold <at> skeeve.com>
To: eggert <at> cs.ucla.edu, arnold <at> skeeve.com, 17056 <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Sun, 23 Mar 2014 22:01:17 +0200

Hi Paul.

> Date: Fri, 21 Mar 2014 11:09:15 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> To: Aharon Robbins <arnold <at> skeeve.com>, 17056 <at> debbugs.gnu.org
> Subject: Re: bug#17056: dfa.c patch for systems with no locale support
>
> That's odd, as DJGPP has <locale.h> which is supposed to declare 
> setlocale and define LC_ALL, according to 
> <http://www.delorie.com/djgpp/doc/libc/libc_705.html>.  Could you 
> explain what goes wrong?

See the original report below.

> Does the following patch fix the problem?  It should isolate the issue 
> better.

I didn't pass it on, since the failure is on LC_ALL.

Arnold

> Date: Tue, 18 Mar 2014 11:54:54 -0700 (PDT)
> Subject: Re: [gawk-devel] beta tar ball
> To: Aharon Robbins <arnold <at> skeeve.com>
>
> Content-Type: text/plain; charset=iso-8859-1
>
> Aharon,
>
> I tried to compile gawk 4.1.0f with DJGPP and? I got the following:
>
> dfa.c: In function 'using_simple_locale':
> dfa.c:823: error: 'LC_ALL' undeclared (first use in this function)
> dfa.c:823: error: (Each undeclared identifier is reported only once
> dfa.c:823: error: for each function it appears in.)
> dfa.c:823: warning: initialization makes pointer from integer without a cast
> make.exe[1]: *** [dfa.o] Error 1
>
> Any ideas?

Message #18 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Aharon Robbins <arnold <at> skeeve.com>, 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Sun, 23 Mar 2014 14:16:47 -0700

[Message part 1 (text/plain, inline)]

Aharon Robbins wrote:

> See the original report below

I downloaded the DJGPP sources.  LC_ALL is defined unless one compiles 
as a freestanding environment, in which case neither LC_ALL nor 
setlocale are declared.  So my guess is that your correspondent is using 
a freestanding environment somehow, and the patch that I suggested 
should work.  It's better to avoid #ifdefs inside functions so I 
installed this into the grep master (see attached) and am marking this 
as done; we can always resurrect it if my guess is wrong.

[0001-dfa-port-to-freestanding-DJGPP-Bug-17056.patch (text/plain, attachment)]

Message #21 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Aharon Robbins <arnold <at> skeeve.com>
To: eggert <at> cs.ucla.edu, arnold <at> skeeve.com, 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Wed, 26 Mar 2014 22:08:39 +0200

Hi Paul.

> Date: Sun, 23 Mar 2014 14:16:47 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> To: Aharon Robbins <arnold <at> skeeve.com>, 17056-done <at> debbugs.gnu.org
> Subject: Re: bug#17056: dfa.c patch for systems with no locale support
>
> Aharon Robbins wrote:
>
> > See the original report below
>
> I downloaded the DJGPP sources.  LC_ALL is defined unless one compiles 
> as a freestanding environment, in which case neither LC_ALL nor 
> setlocale are declared.  So my guess is that your correspondent is using 
> a freestanding environment somehow, and the patch that I suggested 
> should work.  It's better to avoid #ifdefs inside functions so I 
> installed this into the grep master (see attached) and am marking this 
> as done; we can always resurrect it if my guess is wrong.

I don't think your change is right:

	#ifndef LC_ALL
	# define setlocale(category, locale) NULL
	#endif

If setlocale isn't available, we want to assume that we DO have a
unibyte locale, in which case we should be defining this as

	# define setlocale(category, locale) ("C")

No?

I will pass the current dfa.c on to my tester, but I think it's still
not quite right.

Thanks,

Arnold

Message #24 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Aharon Robbins <arnold <at> skeeve.com>, 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Wed, 26 Mar 2014 21:39:48 -0700

[Message part 1 (text/plain, inline)]

Aharon Robbins wrote:
> we should be defining this as
>
> 	# define setlocale(category, locale) ("C")

Thanks, that sounds right.  I installed the attached patch.

[0001-dfa-improve-port-to-freestanding-DJGPP.patch (text/plain, attachment)]

Message #27 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Aharon Robbins <arnold <at> skeeve.com>
To: eggert <at> cs.ucla.edu, arnold <at> skeeve.com, 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Thu, 27 Mar 2014 20:48:38 +0200

Hi Paul.

> Date: Wed, 26 Mar 2014 21:39:48 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> To: Aharon Robbins <arnold <at> skeeve.com>, 17056-done <at> debbugs.gnu.org
>
> Aharon Robbins wrote:
> > we should be defining this as
> >
> > 	# define setlocale(category, locale) ("C")
>
> Thanks, that sounds right.  I installed the attached patch.

Actually, after a night's sleep, I think that was wrong. What if
the system's setlocale() actually returns NULL?  I suggest
the below change to master.

Thanks,

Arnold
----------------------------------------------------------------
diff --git a/src/dfa.c b/src/dfa.c
index f88ff2a..0fd8944 100644
--- a/src/dfa.c
+++ b/src/dfa.c
@@ -37,7 +37,7 @@
 /* Gawk doesn't use Gnulib, so don't assume that setlocale and
    static_assert are present.  */
 #ifndef LC_ALL
-# define setlocale(category, locale) "C"
+# define setlocale(category, locale) NULL
 #endif
 #ifndef static_assert
 # define static_assert(cond, diagnostic) \
@@ -784,8 +784,9 @@ using_simple_locale (void)
       if (unibyte_c < 0)
         {
           char const *locale = setlocale (LC_ALL, NULL);
-          unibyte_c = (locale && (STREQ (locale, "C")
-                                  || STREQ (locale, "POSIX")));
+          unibyte_c = (locale == NULL
+                          || STREQ (locale, "C")
+                          || STREQ (locale, "POSIX"));
         }
       return unibyte_c;
     }

Message #30 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Aharon Robbins <arnold <at> skeeve.com>, 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Thu, 27 Mar 2014 11:57:46 -0700

On 03/27/2014 11:48 AM, Aharon Robbins wrote:
> What if
> the system's setlocale() actually returns NULL?
Then we don't know what the locale is.  Perhaps setlocale ran out of 
memory and so returned NULL.  If so, subsequent calls might use some 
weird Turkish locale with multibyte characters.  So it sounds safer to 
assume the worst in that case.

Message #33 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Aharon Robbins <arnold <at> skeeve.com>,
 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Thu, 27 Mar 2014 13:04:32 -0600

[Message part 1 (text/plain, inline)]

On 03/27/2014 12:57 PM, Paul Eggert wrote:
> On 03/27/2014 11:48 AM, Aharon Robbins wrote:
>> What if
>> the system's setlocale() actually returns NULL?
> Then we don't know what the locale is.  Perhaps setlocale ran out of
> memory and so returned NULL.  If so, subsequent calls might use some
> weird Turkish locale with multibyte characters.  So it sounds safer to
> assume the worst in that case.

This is in main().  POSIX guarantees that until setlocale() is called
for the first time, we are in the C locale.  If you were in a library, I
could buy the argument of setlocale() returning NULL as indicative of
some rare error.  But in main(), where you are the first call, the only
thing that a NULL return implies is that your attempt to change the
locale had no effect, so it remains at the locale it was before, which
is the C locale since all programs start in the C locale.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #36 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Aharon Robbins <arnold <at> skeeve.com>
To: eggert <at> cs.ucla.edu, eblake <at> redhat.com, arnold <at> skeeve.com,
 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Fri, 28 Mar 2014 13:30:54 +0300

Me: 
> >> What if
> >> the system's setlocale() actually returns NULL?

Paul:
> > Then we don't know what the locale is.  Perhaps setlocale ran out of
> > memory and so returned NULL.  If so, subsequent calls might use some
> > weird Turkish locale with multibyte characters.  So it sounds safer to
> > assume the worst in that case.

Eric:
> This is in main().  POSIX guarantees that until setlocale() is called
> for the first time, we are in the C locale.  If you were in a library, I
> could buy the argument of setlocale() returning NULL as indicative of
> some rare error.  But in main(), where you are the first call, the only
> thing that a NULL return implies is that your attempt to change the
> locale had no effect, so it remains at the locale it was before, which
> is the C locale since all programs start in the C locale.

So, are we for or against my suggested change?  Sounds like "for",
but I'm not sure.

Thanks,

Arnold

Message #39 received at 17056-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Aharon Robbins <arnold <at> skeeve.com>, eblake <at> redhat.com, 
 17056-done <at> debbugs.gnu.org
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Fri, 28 Mar 2014 19:28:05 -0700

Aharon Robbins wrote:
> So, are we for or against my suggested change?  Sounds like "for",

Yes, thanks, it looks good.  I installed it.

Message #42 received at 17056 <at> debbugs.gnu.org (full text, mbox):

From: Paolo Bonzini <bonzini <at> gnu.org>
To: 17056 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu, arnold <at> skeeve.com
Subject: Re: bug#17056: dfa.c patch for systems with no locale support
Date: Tue, 01 Apr 2014 10:17:56 +0200

Il 23/03/2014 22:16, Paul Eggert ha scritto:
> Aharon Robbins wrote:
>
>> See the original report below
>
> I downloaded the DJGPP sources.  LC_ALL is defined unless one compiles
> as a freestanding environment, in which case neither LC_ALL nor
> setlocale are declared.  So my guess is that your correspondent is using
> a freestanding environment somehow, and the patch that I suggested
> should work.  It's better to avoid #ifdefs inside functions so I
> installed this into the grep master (see attached) and am marking this
> as done; we can always resurrect it if my guess is wrong.
>

I disagree with this patch.  In grep, the right place to fix this should 
be gnulib.  Unless we get a report about this bug for grep, the patch 
should be installed only in gawk.

Paolo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.