Skip to content

Commit

Permalink
tests: setenv LANG to en_US.UTF-8 in bsdunzip test_I.c
Browse files Browse the repository at this point in the history
  • Loading branch information
@mmatuska
mmatuska committed Apr 30, 2024
1 parent 3604796 commit 83e8b0e
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions unzip/test/test_I.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
DEFINE_TEST(test_I)
{
const char *reffile = "test_I.zip";
const char *lang;
int r;

#if HAVE_SETLOCALE
Expand All @@ -44,11 +45,18 @@ DEFINE_TEST(test_I)
skipping("setlocale() not available on this system.");
#endif

lang = getenv("LANG");
setenv("LANG", "en_US.UTF-8", 1);

This comment has been minimized.

Sign in to view

Copy link
@dag-erling

dag-erling May 1, 2024 •
edited

Contributor

This setenv() call may clobber the memory pointed to by lang.

It is also insufficient, since you don't run in a clean environment, so LANG may be overridden by an inherited LC_ALL or LC_CTYPE, or by the user's .profile (remember that system() does not execute the command directly, but passes it to a shell).

This comment has been minimized.

Sign in to view

Copy link
@mmatuska

mmatuska May 9, 2024 •
edited

Author Member

Would a better solution be to to use this (for non-windows)?

systemf("env LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8 LC_CTYPE=en_US.UTF-8 %s -I UTF-8 %s >test.out 2>test.err", testprog, reffile);

mmatuska@2284338

This comment has been minimized.

Sign in to view

Copy link
@dag-erling

dag-erling May 10, 2024

Contributor

No, this is pointless since LC_ALL overrides both LC_CTYPE and LANG. Ideally you want to clear your environment (env -i) and set only LC_CTYPE, to narrowly target what you're testing. On the other hand, clearing the environment might perturb the test, if for instance you rely on a specific PATH or TMPDIR setting. So perhaps something like env -uLC_ALL LC_CTYPE=en_US.UTF-8. Alternatively, you could use env LC_ALL=C.UTF-8 if all target platforms support that locale (I believe macOS didn't until partway through 13). And you can avoid the #ifdef by manipulating your own environment using setenv() / unsetenv() prior to calling system().

Also, unzip should use setlocale(LC_CTYPE, NULL) instead of setlocale(LC_ALL, NULL) as the latter may have unintended side effects such as changing the behavior of printf(), scanf(), strerror()...
extract_reference_file(reffile);
r = systemf("%s -I UTF-8 %s >test.out 2>test.err", testprog, reffile);
assertEqualInt(0, r);
assertNonEmptyFile("test.out");
assertEmptyFile("test.err");

assertTextFileContents("Hello, World!\n", "Γειά σου Κόσμε.txt");

if (lang == NULL)
unsetenv("LANG");
else
setenv("LANG", lang, 1);
}

0 comments on commit 83e8b0e

Please sign in to comment.