[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-bugs
Subject:    Re: setlocale() bugs
From:       Ingo Schwarze <schwarze () usta ! de>
Date:       2018-03-31 20:08:06
Message-ID: 20180331200806.GA30450 () athene ! usta ! de
[Download RAW message or body]

Hi Karl,

Karl Williamson wrote on Sat, Mar 31, 2018 at 11:50:16AM -0600:
>> Karl Williamson wrote on Mon, Mar 19, 2018 at 11:51:31AM -0600:

>>> (And to be
>>> consistent, LC_ALL should have been the heterogenous composition of all
>>> the locale strings that the program sets, even if they just boil down to
>>> C or UTF-8.  Programs that run on other OS's are expecting this, and
>>> again your portability goal is compromised)

I'm calling this paragraph [*] for later reference.

> If I set LC_CTYPE to foo, and LC_COLLATE to bar, and then request 
> setlocale(LC_CTYPE, NULL), it returns 'foo', and
> setlocale(LC_COLLATE, NULL), it returns 'bar', even though that's not 
> really true.  I understand why you do that, but I claim that you should 
> maintain consistency when asking for
> setlocale(LC_ALL, NULL)
> Currently it just says 'C', but code from other systems would be 
> expecting it to act as if the previous calls actually took effect.
> In Linux it would return
> LC_CTYPE=foo;LC_COLLATE=bar;LC_NUMERIC=C...
> And I think you should do something similar, so that what gets returned 
> by LC_ALL reflects what previous individual setlocale's have done.

That's exactly what i fixed with my recently committed patch,
see the the first part of the test program appended below.

> Right now, if you set LC_CTYPE to foo, then call
> const char * save=setlocale(LC_ALL, NULL);
>   to save things, then change LC_CTYPE to baz, then call
> setlocale(LC_ALL, save)
> printf("%s\n", setlocale(LC_CTYPE,NULL)
> I don't believe it will be foo because your LC_ALL returned a value that 
> didn't contain 'foo'.

That appears to work as expected, too, after my commit,
see the the second part of the test program appended below.

So you think what you meant with [*] is now completely resolved?

> If this isn't clear, let me know.

I *think* i got your points now - it would be appreacited if you could
either confirm that or point me to whatever i'm still missing.

> Your setlocale is a stub, but it should act consistently.

I would call it a stub - it does everything required by the POSIX
standard (modulo the bugs that i just fixed, and any that might
still remain to be fixed).  What it doesn't implement is optional
functionality that we consider harmful because it results, for
example, in unpredictable number parsing, inconsistent error message
output, unsafe character encoding, and the like.  But maybe that's
a matter of opinion.

Thanks for following up on this to this point!
  Ingo


 $ make setlocale 
cc -O2 -pipe    -o setlocale setlocale.c

 $ ./setlocale    
First part of test:
setlocale(LC_CTYPE, foo) = foo
setlocale(LC_COLLATE, bar) = bar
setlocale(LC_ALL, NULL) = bar/foo/C/C/C/C

Second part of test:
setlocale(LC_CTYPE, baz) = baz
setlocale(LC_ALL, NULL) = bar/baz/C/C/C/C
setlocale(LC_ALL, save) = bar/foo/C/C/C/C
setlocale(LC_CTYPE, NULL) = foo

 $ cat setlocale.c
#include <err.h>
#include <locale.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int
main(void)
{
	char *retval, *save;

	puts("First part of test:");

	if ((retval = setlocale(LC_CTYPE, "foo")) == NULL)
		errx(1, "setlocale(LC_CTYPE, foo) failed");
	printf("setlocale(LC_CTYPE, foo) = %s\n", retval);

	if ((retval = setlocale(LC_COLLATE, "bar")) == NULL)
		errx(1, "setlocale(LC_COLLATE, bar) failed");
	printf("setlocale(LC_COLLATE, bar) = %s\n", retval);

	if ((retval = setlocale(LC_ALL, NULL)) == NULL)
		errx(1, "setlocale(LC_ALL, NULL) failed");
	printf("setlocale(LC_ALL, NULL) = %s\n", retval);

	puts("\nSecond part of test:");

	if ((save = strdup(retval)) == NULL)
		err(1, NULL);

	if ((retval = setlocale(LC_CTYPE, "baz")) == NULL)
		errx(1, "setlocale(LC_CTYPE, baz) failed");
	printf("setlocale(LC_CTYPE, baz) = %s\n", retval);

	if ((retval = setlocale(LC_ALL, NULL)) == NULL)
		errx(1, "setlocale(LC_ALL, NULL) failed");
	printf("setlocale(LC_ALL, NULL) = %s\n", retval);

	if ((retval = setlocale(LC_ALL, save)) == NULL)
		errx(1, "setlocale(LC_ALL, save) failed");
	printf("setlocale(LC_ALL, save) = %s\n", retval);

	if ((retval = setlocale(LC_CTYPE, NULL)) == NULL)
		errx(1, "setlocale(LC_CTYPE, NULL) failed");
	printf("setlocale(LC_CTYPE, NULL) = %s\n", retval);

	free(save);
	return 0;
}

[prev in list] [next in list] [prev in thread] [next in thread]