Linux-Sparse Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] test-inspect: reset locale after gtk_init()
@ 2020-07-05 18:50 Davidson Francis
  2020-07-05 19:12 ` Linus Torvalds
  0 siblings, 1 reply; 4+ messages in thread
From: Davidson Francis @ 2020-07-05 18:50 UTC (permalink / raw)
  To: linux-sparse; +Cc: Davidson Francis

The test-inspect tool uses GTK to visualize symbol nodes. It turns
out that gtk_init() implicitly sets the locale to the system locale,
and since Sparse uses strtod()/strtold() for parsing floating-point
numbers in expressions, parsing becomes locale-dependent.

Since the system's locale may be different from "C", test-inspect
may be unable to parse float numbers.

Steps to reproduce:
    $ echo "int main(void){3.14;}" > test.c
    $ LC_ALL="fr_FR.UTF-8" test-inspect test.c
Output:
    test.c:1:16: error: constant 3.14 is not a valid number

Fix this by resetting the locale right after gtk_init().

Signed-off-by: Davidson Francis <davidsondfgl@gmail.com>
---
 test-inspect.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test-inspect.c b/test-inspect.c
index 63754cb3..a59cd902 100644
--- a/test-inspect.c
+++ b/test-inspect.c
@@ -6,6 +6,7 @@
 #include <ctype.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <locale.h>
 
 #include "lib.h"
 #include "allocate.h"
@@ -31,6 +32,7 @@ int main(int argc, char **argv)
 	struct symbol_list *view_syms = NULL;
 
 	gtk_init(&argc,&argv);
+	setlocale(LC_ALL, "C");
 	expand_symbols(sparse_initialize(argc, argv, &filelist));
 	FOR_EACH_PTR(filelist, file) {
 		struct symbol_list *syms = sparse(file);
-- 
2.11.0

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

* Re: [PATCH] test-inspect: reset locale after gtk_init()
  2020-07-05 18:50 [PATCH] test-inspect: reset locale after gtk_init() Davidson Francis
@ 2020-07-05 19:12 ` Linus Torvalds
  2020-07-05 20:35   ` Luc Van Oostenryck
  0 siblings, 1 reply; 4+ messages in thread
From: Linus Torvalds @ 2020-07-05 19:12 UTC (permalink / raw)
  To: Davidson Francis; +Cc: Sparse Mailing-list

On Sun, Jul 5, 2020 at 11:51 AM Davidson Francis <davidsondfgl@gmail.com> wrote:
>
> The test-inspect tool uses GTK to visualize symbol nodes. It turns
> out that gtk_init() implicitly sets the locale to the system locale,
> and since Sparse uses strtod()/strtold() for parsing floating-point
> numbers in expressions, parsing becomes locale-dependent.

We probably shouldn't be using strtod/strtold in the first place
because of issues like this, but I think your patch is likely the
simplest fix.

It _might_ be a good idea to limit it to LC_NUMERIC instead of LC_ALL,
but who knows.. I'm not sure what else might be affected (and I'm not
sure how good LC_NUMERIC support is on all platforms)

               Linus

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

* Re: [PATCH] test-inspect: reset locale after gtk_init()
  2020-07-05 19:12 ` Linus Torvalds
@ 2020-07-05 20:35   ` Luc Van Oostenryck
  2020-07-05 23:28     ` Davidson Francis
  0 siblings, 1 reply; 4+ messages in thread
From: Luc Van Oostenryck @ 2020-07-05 20:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Davidson Francis, Sparse Mailing-list

On Sun, Jul 05, 2020 at 12:12:55PM -0700, Linus Torvalds wrote:
> On Sun, Jul 5, 2020 at 11:51 AM Davidson Francis <davidsondfgl@gmail.com> wrote:
> >
> > The test-inspect tool uses GTK to visualize symbol nodes. It turns
> > out that gtk_init() implicitly sets the locale to the system locale,
> > and since Sparse uses strtod()/strtold() for parsing floating-point
> > numbers in expressions, parsing becomes locale-dependent.

Oh yes, indeed :(
Thanks for the patch.
 
> We probably shouldn't be using strtod/strtold in the first place
> because of issues like this, but I think your patch is likely the
> simplest fix.

Yes. 
 
> It _might_ be a good idea to limit it to LC_NUMERIC instead of LC_ALL,
> but who knows.. I'm not sure what else might be affected (and I'm not
> sure how good LC_NUMERIC support is on all platforms)

Well, checking the standard, I see that strtold() first strips
whitespaces as defined by isspace() and isspace() itself depends
on LC_CTYPE. So, for the moment, I prefer to take Davidson's
patch as is this (OTOH, we already depend on isspace() anyway).

-- Luc

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

* Re: [PATCH] test-inspect: reset locale after gtk_init()
  2020-07-05 20:35   ` Luc Van Oostenryck
@ 2020-07-05 23:28     ` Davidson Francis
  0 siblings, 0 replies; 4+ messages in thread
From: Davidson Francis @ 2020-07-05 23:28 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: Linus Torvalds, Sparse Mailing-list

On Sun, Jul 05, 2020 at 10:35:02PM +0200, Luc Van Oostenryck wrote:
> On Sun, Jul 05, 2020 at 12:12:55PM -0700, Linus Torvalds wrote:
> > It _might_ be a good idea to limit it to LC_NUMERIC instead of
> > LC_ALL,
> > but who knows.. I'm not sure what else might be affected (and I'm
> > not
> > sure how good LC_NUMERIC support is on all platforms)
>
> Well, checking the standard, I see that strtold() first strips
> whitespaces as defined by isspace() and isspace() itself depends
> on LC_CTYPE. So, for the moment, I prefer to take Davidson's
> patch as is this (OTOH, we already depend on isspace() anyway).
>
> -- Luc

I was also in doubt as to what would be the most appropriate
option. Anyway, thanks for accepting the patch.

Regards,
Davidson Francis.

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-05 18:50 [PATCH] test-inspect: reset locale after gtk_init() Davidson Francis
2020-07-05 19:12 ` Linus Torvalds
2020-07-05 20:35   ` Luc Van Oostenryck
2020-07-05 23:28     ` Davidson Francis

Linux-Sparse Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sparse/0 linux-sparse/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sparse linux-sparse/ https://lore.kernel.org/linux-sparse \
		linux-sparse@vger.kernel.org
	public-inbox-index linux-sparse

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sparse


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git