Skip to content

fix(strvec_{posix,str}cmp): use unsigned char to be consistent with strcmp - #1

Open
akinomyoga wants to merge 1 commit into
develfrom
patch-strvec_posixcmp
Open

fix(strvec_{posix,str}cmp): use unsigned char to be consistent with strcmp#1
akinomyoga wants to merge 1 commit into
develfrom
patch-strvec_posixcmp

Conversation

@akinomyoga

Copy link
Copy Markdown
Member

The present code tries to reduce the call to strcmp by comparing the first character of the strings inline, but the comparison is made by char (which is signed in a typical environment). This is inconsistent with strcmp, which compares strings using unsigned char.

I'm not sure if I need to show an actual affected case, but it is hard to create a natural case. I'm not sure if this can be robustly reproducible in other systems, but the following demonstrates the issue in my system (Linux, with strcoll(3) from glibc).

$ mkdir t
$ cd t
$ LANG=C
$ touch {,a}{$'\1',$'\xA9'}.txt
$ LANG=en_US.UTF-8
$ printf'%s\n'*.txt | cat -va^A.txtaM-).txtM-).txt^A.txt

\x01 and \xA9 are shown by ^A and M-) in the cat -v output. Here, we can observe that, when \x01 and \xA9 are the second characters of the filenames, \x01 comes first. However, when they are the first characters of the filenames, \xA9 comes first because it is a negative number (-87) in char.

…onsistent with strcmp
The present code tries to reduce the call to "strcmp" by comparing the
first character of the strings inline, but the comparison is made by
char (which is signed in a typical environment). This is inconsistent
with strcmp, which compares strings using unsigned char.
I'm not sure if I need to show an actual affected case, but it is hard
to create a natural case. I'm not sure if this can be robustly
reproducible in other systems, but the following demonstrates the
issue in my system (Linux, with strcoll(3) from glibc).
$ mkdir t
$ cd t
$ LANG=C
$ touch {,a}{$'\1',$'\xA9'}.txt
$ LANG=en_US.UTF-8
$ printf '%s\n' *.txt | cat -v
a^A.txt
aM-).txt
M-).txt
^A.txt
\x01 and \xA9 are shown by ^A and M-) in the "cat -v" output. Here,
we can observe that, when \x01 and \xA9 are the second characters of
the filenames, \x01 comes first. However, when they are the first
characters of the filenames, \xA9 comes first because it is a negative
number (-87) in char.
@akinomyoga
akinomyogaforce-pushed the patch-strvec_posixcmp branch from 3496d08 to 10642adCompareAugust 13, 2026 21:52
@akinomyogaakinomyoga changed the title lib/sh/stringvec.c (strvec_{posix,str}cmp): use unsigned char to be consistent with strcmpfix(strvec_{posix,str}cmp): use unsigned char to be consistent with strcmpAug 13, 2026
@akinomyoga

Copy link
Copy Markdown
MemberAuthor

Another possibility is to prepare a function for the common part:

staticintstrvec_truestrcmp (char**s1, char**s2)
{
intresult;
if ((result= (unsigned char)**s1- (unsigned char)**s2) ==0)
result=strcmp (*s1, *s2);
returnresult;
}

and make strvec_posixcmp and strvec_strcmp call strvec_truestrcmp.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@akinomyoga