Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20221108032509.GH29905@brightrain.aerifal.cx>
Date: Mon, 7 Nov 2022 22:25:09 -0500
From: Rich Felker <dalias@...c.org>
To: Dmitry Bogatov <oht-tfnfy#tah.bet#i1@...tion.cc>
Cc: bug-gsasl@....org, musl@...ts.openwall.com
Subject: Re: Behaviour of strverscmp(3)

On Mon, Nov 07, 2022 at 10:08:25PM -0500, Rich Felker wrote:
> On Sun, Nov 06, 2022 at 06:39:04PM -0500, Rich Felker wrote:
> > On Sun, Nov 06, 2022 at 06:18:22PM -0500, Dmitry Bogatov wrote:
> > > Hello.
> > > 
> > > While trying to building gsasl statically with musl library as part of
> > > Nixpkgs distribution, I noticed that test built from tests/version.c
> > > fails when built with musl library. After a bit of troubleshooting, I
> > > can pinpoint the reason -- different behaviour of "strverscmp" from
> > > glibc and musl.
> > > 
> > > Example code:
> > > 
> > > #include <string.h>
> > > #include <stdio.h>
> > > 
> > > int main()
> > > {
> > > 	int value = strverscmp("UNKNOWN", "2.2.0");
> > > 	printf("%d\n", value);
> > > 	return 0;
> > > }
> > > 
> > > Under glibc value "35" is printed (positive), under musl value "-1" is
> > > printed (negative). Not sure what is the correct solution for the
> > > issue, so I cross-post into two lists.
> > > 
> > > For now I plan to patch-out this particular test. Thank you.
> > 
> > It looks like we're neglecting to honor the exception case to "longer
> > digit sequence is greater" when one of the sequences is degenerate (no
> > digits).
> 
> I think the attached patch fixes it in the most non-invasive way
> that's most clear in avoiding other unwanted side effects. It
> basically says "only apply the longest-digit-sequence" rule if there
> is a common nonzero length [[:digit:]]+ match (dp is the position
> where digit sequence starts, j is the test position).
> 
> I think this code should be reviewed for additional bugs though.
> 
> Rich

> diff --git a/src/string/strverscmp.c b/src/string/strverscmp.c
> index 4daf276d..9e35422a 100644
> --- a/src/string/strverscmp.c
> +++ b/src/string/strverscmp.c
> @@ -22,8 +22,8 @@ int strverscmp(const char *l0, const char *r0)
>  		/* If we're not looking at a digit sequence that began
>  		 * with a zero, longest digit string is greater. */
>  		for (j=i; isdigit(l[j]); j++)
> -			if (!isdigit(r[j])) return 1;
> -		if (isdigit(r[j])) return -1;
> +			if (dp<j && !isdigit(r[j])) return 1;
> +		if (dp<j && isdigit(r[j])) return -1;
>  	} else if (z && dp<i && (isdigit(l[i]) || isdigit(r[i]))) {
>  		/* Otherwise, if common prefix of digit sequence is
>  		 * all zeros, digits order less than non-digits. */

An alternate way to do this might be changing the condition for the if
block:

-	if (l[dp]!='0' && r[dp]!='0') {
+	if (l[dp]-'1'<9U && r[dp]-'1'<9U) {

That is, only doing the longest-digit-sequence logic at all if both l
and r have a nonzero digit at dp. This is probably more efficient.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.