mailing list of musl libc
 help / color / mirror / code / Atom feed
* Re: memmem() - is it correct?
@ 2014-04-09 12:14 u-igbb
  0 siblings, 0 replies; 6+ messages in thread
From: u-igbb @ 2014-04-09 12:14 UTC (permalink / raw)
  To: musl

On Wed, Apr 09, 2014 at 01:19:03PM +0300, Timo Teras wrote:
> >   const char *haystack = "abcde";
> >   return(!memmem(haystack, 4, "cde", 3));

> > returns 1 (as I would expect it to) if linked against uclibc
> > returns 0                           if linked against musl
> > (on ia32)

> musl looks correct to me. "abcde" is five bytes. You test for 4 bytes

I am testing for 3 bytes "cde" inside 4 bytes "abcd", this should not
be successful.

> so it's infact searching from haystack equivalent of "abcd".

Exactly. It does _not_ contain "cde". This should return NULL
from the memmem() and hence 1 from main().

> I'd say uclibc is off-by-one, and broken.

I guess you misinterpreted the test code, there is a '!' which transforms
a returned pointer (success) to 0 exit status in main() and vice versa.

Rune



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

* Re: memmem() - is it correct?
  2014-04-09 12:54       ` Timo Teras
@ 2014-04-10  1:15         ` Rich Felker
  0 siblings, 0 replies; 6+ messages in thread
From: Rich Felker @ 2014-04-10  1:15 UTC (permalink / raw)
  To: musl; +Cc: u-igbb

On Wed, Apr 09, 2014 at 03:54:03PM +0300, Timo Teras wrote:
> On Wed, 9 Apr 2014 15:51:16 +0300
> Timo Teras <timo.teras@iki.fi> wrote:
> 
> > On Wed, 9 Apr 2014 12:49:25 +0200
> > u-igbb@aetey.se wrote:
> > 
> > > On Wed, Apr 09, 2014 at 01:19:03PM +0300, Timo Teras wrote:
> > > > >   const char *haystack = "abcde";
> > > > >   return(!memmem(haystack, 4, "cde", 3));
> > > 
> > > > > returns 1 (as I would expect it to) if linked against uclibc
> > > > > returns 0                           if linked against musl
> > > > > (on ia32)
> > > 
> > > I guess you misinterpreted the test code, there is a '!' which
> > > transforms a returned pointer (success) to 0 exit status in main()
> > > and vice versa.
> > 
> > Right. Should have read it more carefully.  Yes, looks like musl bug.
> > 
> > Perhaps something like the following is in place:
> 
> Wrong patch version. Should be as simple as:
> 
> diff --git a/src/string/memmem.c b/src/string/memmem.c
> index 5211d75..1173020 100644
> --- a/src/string/memmem.c
> +++ b/src/string/memmem.c
> @@ -139,6 +139,7 @@ void *memmem(const void *h0, size_t k, const void
> *n0, size_t l) /* Use faster algorithms for short needles */
>  	h = memchr(h0, *n, k);
>  	if (!h || l==1) return (void *)h;
> +	k -= h - (const unsigned char*)h0;
>  	if (l==2) return twobyte_memmem(h, k, n);
>  	if (l==3) return threebyte_memmem(h, k, n);
>  	if (l==4) return fourbyte_memmem(h, k, n);

Thanks! Committed.

Rich


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

* Re: memmem() - is it correct?
  2014-04-09 12:51     ` Timo Teras
@ 2014-04-09 12:54       ` Timo Teras
  2014-04-10  1:15         ` Rich Felker
  0 siblings, 1 reply; 6+ messages in thread
From: Timo Teras @ 2014-04-09 12:54 UTC (permalink / raw)
  To: u-igbb; +Cc: musl

On Wed, 9 Apr 2014 15:51:16 +0300
Timo Teras <timo.teras@iki.fi> wrote:

> On Wed, 9 Apr 2014 12:49:25 +0200
> u-igbb@aetey.se wrote:
> 
> > On Wed, Apr 09, 2014 at 01:19:03PM +0300, Timo Teras wrote:
> > > >   const char *haystack = "abcde";
> > > >   return(!memmem(haystack, 4, "cde", 3));
> > 
> > > > returns 1 (as I would expect it to) if linked against uclibc
> > > > returns 0                           if linked against musl
> > > > (on ia32)
> > 
> > I guess you misinterpreted the test code, there is a '!' which
> > transforms a returned pointer (success) to 0 exit status in main()
> > and vice versa.
> 
> Right. Should have read it more carefully.  Yes, looks like musl bug.
> 
> Perhaps something like the following is in place:

Wrong patch version. Should be as simple as:

diff --git a/src/string/memmem.c b/src/string/memmem.c
index 5211d75..1173020 100644
--- a/src/string/memmem.c
+++ b/src/string/memmem.c
@@ -139,6 +139,7 @@ void *memmem(const void *h0, size_t k, const void
*n0, size_t l) /* Use faster algorithms for short needles */
 	h = memchr(h0, *n, k);
 	if (!h || l==1) return (void *)h;
+	k -= h - (const unsigned char*)h0;
 	if (l==2) return twobyte_memmem(h, k, n);
 	if (l==3) return threebyte_memmem(h, k, n);
 	if (l==4) return fourbyte_memmem(h, k, n);


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

* Re: memmem() - is it correct?
       [not found]   ` <20140409104925.GE21662@example.net>
@ 2014-04-09 12:51     ` Timo Teras
  2014-04-09 12:54       ` Timo Teras
  0 siblings, 1 reply; 6+ messages in thread
From: Timo Teras @ 2014-04-09 12:51 UTC (permalink / raw)
  To: u-igbb; +Cc: musl

On Wed, 9 Apr 2014 12:49:25 +0200
u-igbb@aetey.se wrote:

> On Wed, Apr 09, 2014 at 01:19:03PM +0300, Timo Teras wrote:
> > >   const char *haystack = "abcde";
> > >   return(!memmem(haystack, 4, "cde", 3));
> 
> > > returns 1 (as I would expect it to) if linked against uclibc
> > > returns 0                           if linked against musl
> > > (on ia32)
> 
> I guess you misinterpreted the test code, there is a '!' which
> transforms a returned pointer (success) to 0 exit status in main()
> and vice versa.

Right. Should have read it more carefully.  Yes, looks like musl bug.

Perhaps something like the following is in place:

diff --git a/src/string/memmem.c b/src/string/memmem.c
index 5211d75..0594fea 100644
--- a/src/string/memmem.c
+++ b/src/string/memmem.c
@@ -138,7 +138,9 @@ void *memmem(const void *h0, size_t k, const void
*n0, size_t l) 
 	/* Use faster algorithms for short needles */
 	h = memchr(h0, *n, k);
-	if (!h || l==1) return (void *)h;
+	if (!h) return 0;
+	if (l==1) return (void *)h;
+	k -= h - h0;
 	if (l==2) return twobyte_memmem(h, k, n);
 	if (l==3) return threebyte_memmem(h, k, n);
 	if (l==4) return fourbyte_memmem(h, k, n);



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

* Re: memmem() - is it correct?
  2014-04-09 10:08 u-igbb
@ 2014-04-09 10:19 ` Timo Teras
       [not found]   ` <20140409104925.GE21662@example.net>
  0 siblings, 1 reply; 6+ messages in thread
From: Timo Teras @ 2014-04-09 10:19 UTC (permalink / raw)
  To: musl; +Cc: u-igbb

On Wed, 9 Apr 2014 12:08:40 +0200
u-igbb@aetey.se wrote:

> A test case adapted from elsewhere:
> ----
> #include <string.h>
> 
> main(){
>   const char *haystack = "abcde";
>   return(!memmem(haystack, 4, "cde", 3));
> }
> ----
> 
> returns 1 (as I would expect it to) if linked against uclibc
> returns 0                           if linked against musl
> (on ia32)
> 
> Any comments?

musl looks correct to me. "abcde" is five bytes. You test for 4 bytes
so it's infact searching from haystack equivalent of "abcd".

I'd say uclibc is off-by-one, and broken.

- Timo


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

* memmem() - is it correct?
@ 2014-04-09 10:08 u-igbb
  2014-04-09 10:19 ` Timo Teras
  0 siblings, 1 reply; 6+ messages in thread
From: u-igbb @ 2014-04-09 10:08 UTC (permalink / raw)
  To: musl

A test case adapted from elsewhere:
----
#include <string.h>

main(){
  const char *haystack = "abcde";
  return(!memmem(haystack, 4, "cde", 3));
}
----

returns 1 (as I would expect it to) if linked against uclibc
returns 0                           if linked against musl
(on ia32)

Any comments?
Rune



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

end of thread, other threads:[~2014-04-10  1:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-09 12:14 memmem() - is it correct? u-igbb
  -- strict thread matches above, loose matches on Subject: below --
2014-04-09 10:08 u-igbb
2014-04-09 10:19 ` Timo Teras
     [not found]   ` <20140409104925.GE21662@example.net>
2014-04-09 12:51     ` Timo Teras
2014-04-09 12:54       ` Timo Teras
2014-04-10  1:15         ` Rich Felker

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