dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
@ 2014-08-05 16:40 Chema Gonzalez
  2014-08-05 17:00 ` Jérémie Courrèges-Anglas
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Chema Gonzalez @ 2014-08-05 16:40 UTC (permalink / raw)
  To: Herbert Xu; +Cc: dash, Chema Gonzalez

If the user environment has either LC_ALL or LANG defined, the setting
of LC_COLLATE in src/mkbuiltins is overriden. With a non-POSIX locale,
the orders of dotcmd (remember that '.' is 0x2e in ascii) and truecmd
(':' is 0x3a in ascii) are reversed, which makes the ":" command fail
in the bsearch.

Tested:

Before this patch:

$ env |grep -e LANG -e LC_ALL
LC_ALL=en_US.ISO8859-15
LANG=en_US.iso885915
$ ./autogen.sh
...
$ ./configure
...
$ make clean; make -j 40
...
$ ./src/dash -c ":"
./src/dash: 1: :: not found
$ grep -A 3 'struct builtincmd builtincmd' src/builtins.c
const struct builtincmd builtincmd[] = {
        { ":", truecmd, 3 },
        { ".", dotcmd, 3 },
        { "[", testcmd, 0 },
$ make clean; LC_ALL= LANG= make -j 40
...
$ ./src/dash -c ":"
$ grep -A 3 'struct builtincmd builtincmd' src/builtins.c
const struct builtincmd builtincmd[] = {
        { ".", dotcmd, 3 },
        { ":", truecmd, 3 },
        { "[", testcmd, 0 },

After this patch:

env |grep -e LANG -e LC_ALL
LC_ALL=en_US.ISO8859-15
LANG=en_US.iso885915
$ ./autogen.sh
$ ./configure
...
$ make clean; make -j 40
...
$ ./src/dash -c ":"
$ grep -A 3 'struct builtincmd builtincmd' src/builtins.c
const struct builtincmd builtincmd[] = {
        { ".", dotcmd, 3 },
        { ":", truecmd, 3 },
        { "[", testcmd, 0 },

Signed-off-by: Chema Gonzalez <chema@google.com>
---
 src/mkbuiltins | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mkbuiltins b/src/mkbuiltins
index f562ae2..ed79041 100644
--- a/src/mkbuiltins
+++ b/src/mkbuiltins
@@ -78,7 +78,7 @@ awk '{	for (i = 2 ; i <= NF ; i++) {
 		if ($i ~ /^-/)
 			line = $(++i) "\t" line
 		print line
-	}}' $temp | LC_COLLATE=C sort -k 1,1 | tee $temp2 | awk '{
+	}}' $temp | LC_ALL= LANG= LC_COLLATE=C sort -k 1,1 | tee $temp2 | awk '{
 		opt = ""
 		if (NF > 2) {
 			opt = substr($2, 2)
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 16:40 [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden Chema Gonzalez
@ 2014-08-05 17:00 ` Jérémie Courrèges-Anglas
  2014-08-05 17:11   ` Chema Gonzalez
  2014-08-05 17:09 ` Eric Blake
  2014-08-22 23:08 ` [PATCH dash v2] " Chema Gonzalez
  2 siblings, 1 reply; 10+ messages in thread
From: Jérémie Courrèges-Anglas @ 2014-08-05 17:00 UTC (permalink / raw)
  To: Chema Gonzalez; +Cc: Herbert Xu, dash

Chema Gonzalez <chema@google.com> writes:

> If the user environment has either LC_ALL or LANG defined, the setting
> of LC_COLLATE in src/mkbuiltins is overriden. With a non-POSIX locale,
> the orders of dotcmd (remember that '.' is 0x2e in ascii) and truecmd
> (':' is 0x3a in ascii) are reversed, which makes the ":" command fail
> in the bsearch.

Actually the problem should only occur when you set LC_ALL, which
overrides LANG and LC_COLLATE.  Even though some may find nice to
protect the unwary, but I'd expect lots of brokenness in this unusual
build situation, and this not limited to dash.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 16:40 [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden Chema Gonzalez
  2014-08-05 17:00 ` Jérémie Courrèges-Anglas
@ 2014-08-05 17:09 ` Eric Blake
  2014-08-05 17:12   ` Chema Gonzalez
  2014-08-22 23:08 ` [PATCH dash v2] " Chema Gonzalez
  2 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2014-08-05 17:09 UTC (permalink / raw)
  To: Chema Gonzalez, Herbert Xu; +Cc: dash

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]

On 08/05/2014 10:40 AM, Chema Gonzalez wrote:
> If the user environment has either LC_ALL or LANG defined, the setting
> of LC_COLLATE in src/mkbuiltins is overriden. With a non-POSIX locale,
> the orders of dotcmd (remember that '.' is 0x2e in ascii) and truecmd
> (':' is 0x3a in ascii) are reversed, which makes the ":" command fail
> in the bsearch.
> 

> -	}}' $temp | LC_COLLATE=C sort -k 1,1 | tee $temp2 | awk '{
> +	}}' $temp | LC_ALL= LANG= LC_COLLATE=C sort -k 1,1 | tee $temp2 | awk '{

Setting LC_ALL= to the empty string risks implementation-defined
behavior.  Also, LC_ALL overrides LANG and LC_COLLATE.  It should be
sufficient to merely do:

	}}' $temp | LC_ALL=C sort -k 1,1 | tee $temp2 | awk '{

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 17:00 ` Jérémie Courrèges-Anglas
@ 2014-08-05 17:11   ` Chema Gonzalez
  0 siblings, 0 replies; 10+ messages in thread
From: Chema Gonzalez @ 2014-08-05 17:11 UTC (permalink / raw)
  To: Jérémie Courrèges-Anglas; +Cc: Herbert Xu, dash

On Tue, Aug 5, 2014 at 10:00 AM, Jérémie Courrèges-Anglas
<jca+dash@wxcvbn.org> wrote:
> Actually the problem should only occur when you set LC_ALL, which
> overrides LANG and LC_COLLATE.  Even though some may find nice to
My FreeBSD notes (they may be really stale) state LANG overrides
LC_ALL in FreeBSD (4.x). If that's correct, we need to take care of
both.

> protect the unwary, but I'd expect lots of brokenness in this unusual
> build situation, and this not limited to dash.

-Chema


>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 17:09 ` Eric Blake
@ 2014-08-05 17:12   ` Chema Gonzalez
  2014-08-05 17:14     ` Eric Blake
  0 siblings, 1 reply; 10+ messages in thread
From: Chema Gonzalez @ 2014-08-05 17:12 UTC (permalink / raw)
  To: Eric Blake; +Cc: Herbert Xu, dash

On Tue, Aug 5, 2014 at 10:09 AM, Eric Blake <eblake@redhat.com> wrote:
> On 08/05/2014 10:40 AM, Chema Gonzalez wrote:
>> If the user environment has either LC_ALL or LANG defined, the setting
>> of LC_COLLATE in src/mkbuiltins is overriden. With a non-POSIX locale,
>> the orders of dotcmd (remember that '.' is 0x2e in ascii) and truecmd
>> (':' is 0x3a in ascii) are reversed, which makes the ":" command fail
>> in the bsearch.
>>
>
>> -     }}' $temp | LC_COLLATE=C sort -k 1,1 | tee $temp2 | awk '{
>> +     }}' $temp | LC_ALL= LANG= LC_COLLATE=C sort -k 1,1 | tee $temp2 | awk '{
>
> Setting LC_ALL= to the empty string risks implementation-defined
> behavior.  Also, LC_ALL overrides LANG and LC_COLLATE.  It should be
> sufficient to merely do:
>
>         }}' $temp | LC_ALL=C sort -k 1,1 | tee $temp2 | awk '{

Maybe:
    }}' $temp | LC_ALL=C LANG=C sort -k 1,1 | tee $temp2 | awk '{

-Chema

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 17:12   ` Chema Gonzalez
@ 2014-08-05 17:14     ` Eric Blake
  2014-08-05 17:19       ` Chema Gonzalez
  2014-08-22 23:08       ` Chema Gonzalez
  0 siblings, 2 replies; 10+ messages in thread
From: Eric Blake @ 2014-08-05 17:14 UTC (permalink / raw)
  To: Chema Gonzalez; +Cc: Herbert Xu, dash

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

On 08/05/2014 11:12 AM, Chema Gonzalez wrote:
>> Setting LC_ALL= to the empty string risks implementation-defined
>> behavior.  Also, LC_ALL overrides LANG and LC_COLLATE.  It should be
>> sufficient to merely do:
>>
>>         }}' $temp | LC_ALL=C sort -k 1,1 | tee $temp2 | awk '{
> 
> Maybe:
>     }}' $temp | LC_ALL=C LANG=C sort -k 1,1 | tee $temp2 | awk '{

No need to specify LANG=C when LC_ALL is set.  I stand by my shorter line.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 17:14     ` Eric Blake
@ 2014-08-05 17:19       ` Chema Gonzalez
  2014-08-05 17:30         ` Eric Blake
  2014-08-22 23:08       ` Chema Gonzalez
  1 sibling, 1 reply; 10+ messages in thread
From: Chema Gonzalez @ 2014-08-05 17:19 UTC (permalink / raw)
  To: Eric Blake; +Cc: Herbert Xu, dash

On Tue, Aug 5, 2014 at 10:14 AM, Eric Blake <eblake@redhat.com> wrote:
> On 08/05/2014 11:12 AM, Chema Gonzalez wrote:
>>> Setting LC_ALL= to the empty string risks implementation-defined
>>> behavior.  Also, LC_ALL overrides LANG and LC_COLLATE.  It should be
>>> sufficient to merely do:
>>>
>>>         }}' $temp | LC_ALL=C sort -k 1,1 | tee $temp2 | awk '{
>>
>> Maybe:
>>     }}' $temp | LC_ALL=C LANG=C sort -k 1,1 | tee $temp2 | awk '{
>
> No need to specify LANG=C when LC_ALL is set.  I stand by my shorter line.
I'd prefer to see if any FreeBSD user can confirm whether LC_ALL
overrides LANG (as in Linux) or the other way around. The docs seems
to suggest to use LANG instead of LC_ALL there.

http://www.freebsd.org/doc/handbook/using-localization.html

-Chema

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 17:19       ` Chema Gonzalez
@ 2014-08-05 17:30         ` Eric Blake
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2014-08-05 17:30 UTC (permalink / raw)
  To: Chema Gonzalez; +Cc: Herbert Xu, dash

[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]

On 08/05/2014 11:19 AM, Chema Gonzalez wrote:
> On Tue, Aug 5, 2014 at 10:14 AM, Eric Blake <eblake@redhat.com> wrote:
>> On 08/05/2014 11:12 AM, Chema Gonzalez wrote:
>>>> Setting LC_ALL= to the empty string risks implementation-defined
>>>> behavior.  Also, LC_ALL overrides LANG and LC_COLLATE.  It should be
>>>> sufficient to merely do:
>>>>
>>>>         }}' $temp | LC_ALL=C sort -k 1,1 | tee $temp2 | awk '{
>>>
>>> Maybe:
>>>     }}' $temp | LC_ALL=C LANG=C sort -k 1,1 | tee $temp2 | awk '{
>>
>> No need to specify LANG=C when LC_ALL is set.  I stand by my shorter line.
> I'd prefer to see if any FreeBSD user can confirm whether LC_ALL
> overrides LANG (as in Linux) or the other way around. The docs seems
> to suggest to use LANG instead of LC_ALL there.

Bug in the docs, then, as POSIX is quite clear that LC_ALL overrides
LC_* overrides LANG.  If FreeBSD ever got it backwards, even
temporarily, from what POSIX requires, it would be a lot more obvious.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH dash v2] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 16:40 [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden Chema Gonzalez
  2014-08-05 17:00 ` Jérémie Courrèges-Anglas
  2014-08-05 17:09 ` Eric Blake
@ 2014-08-22 23:08 ` Chema Gonzalez
  2 siblings, 0 replies; 10+ messages in thread
From: Chema Gonzalez @ 2014-08-22 23:08 UTC (permalink / raw)
  To: Eric Blake; +Cc: Herbert Xu, Jeremie, dash, Chema Gonzalez

If the user environment has either LC_ALL or LANG defined, the setting
of LC_COLLATE in src/mkbuiltins is overriden. With a non-POSIX locale,
the orders of dotcmd (remember that '.' is 0x2e in ascii) and truecmd
(':' is 0x3a in ascii) are reversed, which makes the ":" command fail
in the bsearch.

Tested:

Before this patch:

$ env |grep -e LANG -e LC_ALL
LC_ALL=en_US.ISO8859-15
LANG=en_US.iso885915
$ ./autogen.sh
...
$ ./configure
...
$ make clean; make -j 40
...
$ ./src/dash -c ":"
./src/dash: 1: :: not found
$ grep -A 3 'struct builtincmd builtincmd' src/builtins.c
const struct builtincmd builtincmd[] = {
        { ":", truecmd, 3 },
        { ".", dotcmd, 3 },
        { "[", testcmd, 0 },
$ make clean; LC_ALL= LANG= make -j 40
...
$ ./src/dash -c ":"
$ grep -A 3 'struct builtincmd builtincmd' src/builtins.c
const struct builtincmd builtincmd[] = {
        { ".", dotcmd, 3 },
        { ":", truecmd, 3 },
        { "[", testcmd, 0 },

After this patch:

$ env |grep -e LANG -e LC_ALL
LC_ALL=en_US.ISO8859-15
LANG=en_US.iso885915
$ ./autogen.sh
$ ./configure
...
$ make clean; make -j 40
...
$ ./src/dash -c ":"
$ grep -A 3 'struct builtincmd builtincmd' src/builtins.c
const struct builtincmd builtincmd[] = {
        { ".", dotcmd, 3 },
        { ":", truecmd, 3 },
        { "[", testcmd, 0 },

Signed-off-by: Chema Gonzalez <chema@google.com>
---
 src/mkbuiltins | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mkbuiltins b/src/mkbuiltins
index f562ae2..70308bd 100644
--- a/src/mkbuiltins
+++ b/src/mkbuiltins
@@ -78,7 +78,7 @@ awk '{	for (i = 2 ; i <= NF ; i++) {
 		if ($i ~ /^-/)
 			line = $(++i) "\t" line
 		print line
-	}}' $temp | LC_COLLATE=C sort -k 1,1 | tee $temp2 | awk '{
+	}}' $temp | LC_ALL=C sort -k 1,1 | tee $temp2 | awk '{
 		opt = ""
 		if (NF > 2) {
 			opt = substr($2, 2)
-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden
  2014-08-05 17:14     ` Eric Blake
  2014-08-05 17:19       ` Chema Gonzalez
@ 2014-08-22 23:08       ` Chema Gonzalez
  1 sibling, 0 replies; 10+ messages in thread
From: Chema Gonzalez @ 2014-08-22 23:08 UTC (permalink / raw)
  To: Eric Blake; +Cc: Herbert Xu, dash

I finally managed to test a FreeBSD 10.0 host, and it seems to respect
LC_ALL before LANG.

Next patch includes just setting LC_ALL (which trumps LC_COLLATE).

-Chema


On Tue, Aug 5, 2014 at 10:14 AM, Eric Blake <eblake@redhat.com> wrote:
> On 08/05/2014 11:12 AM, Chema Gonzalez wrote:
>>> Setting LC_ALL= to the empty string risks implementation-defined
>>> behavior.  Also, LC_ALL overrides LANG and LC_COLLATE.  It should be
>>> sufficient to merely do:
>>>
>>>         }}' $temp | LC_ALL=C sort -k 1,1 | tee $temp2 | awk '{
>>
>> Maybe:
>>     }}' $temp | LC_ALL=C LANG=C sort -k 1,1 | tee $temp2 | awk '{
>
> No need to specify LANG=C when LC_ALL is set.  I stand by my shorter line.
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-08-22 23:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-05 16:40 [PATCH dash] [BUILTIN] ensure LC_COLLATE is not overriden Chema Gonzalez
2014-08-05 17:00 ` Jérémie Courrèges-Anglas
2014-08-05 17:11   ` Chema Gonzalez
2014-08-05 17:09 ` Eric Blake
2014-08-05 17:12   ` Chema Gonzalez
2014-08-05 17:14     ` Eric Blake
2014-08-05 17:19       ` Chema Gonzalez
2014-08-05 17:30         ` Eric Blake
2014-08-22 23:08       ` Chema Gonzalez
2014-08-22 23:08 ` [PATCH dash v2] " Chema Gonzalez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).