mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Behaviour of strverscmp(3)
@ 2022-11-06 23:18 Dmitry Bogatov
  2022-11-06 23:39 ` Rich Felker
       [not found] ` <20221106233904.GE29905__8136.83224130131$1667785799$gmane$org@brightrain.aerifal.cx>
  0 siblings, 2 replies; 5+ messages in thread
From: Dmitry Bogatov @ 2022-11-06 23:18 UTC (permalink / raw)
  To: bug-gsasl, musl

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.

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

* Re: [musl] Behaviour of strverscmp(3)
  2022-11-06 23:18 [musl] Behaviour of strverscmp(3) Dmitry Bogatov
@ 2022-11-06 23:39 ` Rich Felker
  2022-11-08  3:08   ` Rich Felker
       [not found] ` <20221106233904.GE29905__8136.83224130131$1667785799$gmane$org@brightrain.aerifal.cx>
  1 sibling, 1 reply; 5+ messages in thread
From: Rich Felker @ 2022-11-06 23:39 UTC (permalink / raw)
  To: Dmitry Bogatov; +Cc: bug-gsasl, musl

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).

Rich

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

* Re: [musl] Behaviour of strverscmp(3)
       [not found] ` <20221106233904.GE29905__8136.83224130131$1667785799$gmane$org@brightrain.aerifal.cx>
@ 2022-11-07  8:51   ` Simon Josefsson
  0 siblings, 0 replies; 5+ messages in thread
From: Simon Josefsson @ 2022-11-07  8:51 UTC (permalink / raw)
  To: Rich Felker; +Cc: bug-gsasl, musl

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

Rich Felker <dalias@libc.org> writes:

> 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).

Thanks for the report Dmitry (dropping cc because your email adressed
was reject by exim -- '550 restricted characters in address').

Right, I think this is a musl bug that gnulib doesn't detect and work
around.  I ran into it earlier on Alpine, but must have forgotten to
report it (or I found earlier bug reports about it).  Ignoring the
self-test failure is the right solution.  I believe strverscmp is a
glibc-specified interface, so musl should be compatible with it, right?

I'm inclined to add this test vector to the gnulib self-test for
strverscmp, so at least we know where the problem comes from.  Detecting
the issue and working around it seems like a lot of complexity (which is
a source for other bugs) and requires time and energy to support, for
what appears to be little gain, so I'm not sure it is worth it.

/Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 255 bytes --]

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

* Re: [musl] Behaviour of strverscmp(3)
  2022-11-06 23:39 ` Rich Felker
@ 2022-11-08  3:08   ` Rich Felker
  2022-11-08  3:25     ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Rich Felker @ 2022-11-08  3:08 UTC (permalink / raw)
  To: Dmitry Bogatov; +Cc: bug-gsasl, musl

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

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

[-- Attachment #2: strverscmp.diff --]
[-- Type: text/plain, Size: 688 bytes --]

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. */

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

* Re: [musl] Behaviour of strverscmp(3)
  2022-11-08  3:08   ` Rich Felker
@ 2022-11-08  3:25     ` Rich Felker
  0 siblings, 0 replies; 5+ messages in thread
From: Rich Felker @ 2022-11-08  3:25 UTC (permalink / raw)
  To: Dmitry Bogatov; +Cc: bug-gsasl, musl

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

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

end of thread, other threads:[~2022-11-08  3:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-06 23:18 [musl] Behaviour of strverscmp(3) Dmitry Bogatov
2022-11-06 23:39 ` Rich Felker
2022-11-08  3:08   ` Rich Felker
2022-11-08  3:25     ` Rich Felker
     [not found] ` <20221106233904.GE29905__8136.83224130131$1667785799$gmane$org@brightrain.aerifal.cx>
2022-11-07  8:51   ` Simon Josefsson

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).