[PATCH] Modify strnstr.c.

[PATCH] Modify strnstr.c.

[Sichen Zhao]

---
 newlib/libc/string/strnstr.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/newlib/libc/string/strnstr.c b/newlib/libc/string/strnstr.c
index da5e5bd..7c87bbd 100644
--- a/newlib/libc/string/strnstr.c
+++ b/newlib/libc/string/strnstr.c
@@ -44,22 +44,14 @@ __FBSDID("$FreeBSD: head/lib/libc/string/strnstr.c 251069 2013-05-28 20:57:40Z e
  * first slen characters of s.
  */
 char *
-strnstr(const char *s, const char *find, size_t slen)
+strnstr(const char *haystack, const char *needle, size_t haystack_len)
 {
-	char c, sc;
-	size_t len;
+  size_t needle_len = strnlen(needle, haystack_len);
 
-	if ((c = *find++) != '\0') {
-		len = strlen(find);
-		do {
-			do {
-				if (slen-- < 1 || (sc = *s++) == '\0')
-					return (NULL);
-			} while (sc != c);
-			if (len > slen)
-				return (NULL);
-		} while (strncmp(s, find, len) != 0);
-		s--;
-	}
-	return ((char *)s);
+  if (needle_len < haystack_len || !needle[needle_len]) {
+    char *x = memmem(haystack, haystack_len, needle, needle_len);
+    if (x && !memchr(haystack, 0, x - haystack))
+      return x;
+  }
+  return NULL;
 }
-- 
2.7.4

Re: [PATCH] Modify strnstr.c.

[Corinna Vinschen]

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

On Aug 26 21:28, Sichen Zhao wrote:
> ---
>  newlib/libc/string/strnstr.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/newlib/libc/string/strnstr.c b/newlib/libc/string/strnstr.c
> index da5e5bd..7c87bbd 100644
> --- a/newlib/libc/string/strnstr.c
> +++ b/newlib/libc/string/strnstr.c
> @@ -44,22 +44,14 @@ __FBSDID("$FreeBSD: head/lib/libc/string/strnstr.c 251069 2013-05-28 20:57:40Z e
>   * first slen characters of s.
>   */
>  char *
> -strnstr(const char *s, const char *find, size_t slen)
> +strnstr(const char *haystack, const char *needle, size_t haystack_len)
>  {
> -	char c, sc;
> -	size_t len;
> +  size_t needle_len = strnlen(needle, haystack_len);
>  
> -	if ((c = *find++) != '\0') {
> -		len = strlen(find);
> -		do {
> -			do {
> -				if (slen-- < 1 || (sc = *s++) == '\0')
> -					return (NULL);
> -			} while (sc != c);
> -			if (len > slen)
> -				return (NULL);
> -		} while (strncmp(s, find, len) != 0);
> -		s--;
> -	}
> -	return ((char *)s);
> +  if (needle_len < haystack_len || !needle[needle_len]) {
> +    char *x = memmem(haystack, haystack_len, needle, needle_len);
> +    if (x && !memchr(haystack, 0, x - haystack))
> +      return x;
> +  }
> +  return NULL;
>  }
> -- 
> 2.7.4

Input needed:

Given this is a complete reimplementation not based on FreeBSD code at
all, and given the code has been contributed by Eric, shouldn't we
remove the entire copyright header and move this under newlib default
licensing?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

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

Re: [PATCH] Modify strnstr.c.

[Sichen Zhao]

> On Aug 26 21:28, Sichen Zhao wrote:
>> ---
>>   newlib/libc/string/strnstr.c | 24 ++++++++----------------
>>   1 file changed, 8 insertions(+), 16 deletions(-)
>>
>> diff --git a/newlib/libc/string/strnstr.c b/newlib/libc/string/strnstr.c
>> index da5e5bd..7c87bbd 100644
>> --- a/newlib/libc/string/strnstr.c
>> +++ b/newlib/libc/string/strnstr.c
>> @@ -44,22 +44,14 @@ __FBSDID("$FreeBSD: head/lib/libc/string/strnstr.c 251069 2013-05-28 20:57:40Z e
>>    * first slen characters of s.
>>    */
>>   char *
>> -strnstr(const char *s, const char *find, size_t slen)
>> +strnstr(const char *haystack, const char *needle, size_t haystack_len)
>>   {
>> -	char c, sc;
>> -	size_t len;
>> +  size_t needle_len = strnlen(needle, haystack_len);
>>   
>> -	if ((c = *find++) != '\0') {
>> -		len = strlen(find);
>> -		do {
>> -			do {
>> -				if (slen-- < 1 || (sc = *s++) == '\0')
>> -					return (NULL);
>> -			} while (sc != c);
>> -			if (len > slen)
>> -				return (NULL);
>> -		} while (strncmp(s, find, len) != 0);
>> -		s--;
>> -	}
>> -	return ((char *)s);
>> +  if (needle_len < haystack_len || !needle[needle_len]) {
>> +    char *x = memmem(haystack, haystack_len, needle, needle_len);
>> +    if (x && !memchr(haystack, 0, x - haystack))
>> +      return x;
>> +  }
>> +  return NULL;
>>   }
>> -- 
>> 2.7.4
> Input needed:
>
> Given this is a complete reimplementation not based on FreeBSD code at
> all, and given the code has been contributed by Eric, shouldn't we
> remove the entire copyright header and move this under newlib default
> licensing?
Ohk, i see.
>
> Thanks,
> Corinna
>

Re: [PATCH] Modify strnstr.c.

[Sichen Zhao]

> On Aug 26 21:28, Sichen Zhao wrote:
>> ---
>>   newlib/libc/string/strnstr.c | 24 ++++++++----------------
>>   1 file changed, 8 insertions(+), 16 deletions(-)
>>
>> diff --git a/newlib/libc/string/strnstr.c b/newlib/libc/string/strnstr.c
>> index da5e5bd..7c87bbd 100644
>> --- a/newlib/libc/string/strnstr.c
>> +++ b/newlib/libc/string/strnstr.c
>> @@ -44,22 +44,14 @@ __FBSDID("$FreeBSD: head/lib/libc/string/strnstr.c 251069 2013-05-28 20:57:40Z e
>>    * first slen characters of s.
>>    */
>>   char *
>> -strnstr(const char *s, const char *find, size_t slen)
>> +strnstr(const char *haystack, const char *needle, size_t haystack_len)
>>   {
>> -	char c, sc;
>> -	size_t len;
>> +  size_t needle_len = strnlen(needle, haystack_len);
>>   
>> -	if ((c = *find++) != '\0') {
>> -		len = strlen(find);
>> -		do {
>> -			do {
>> -				if (slen-- < 1 || (sc = *s++) == '\0')
>> -					return (NULL);
>> -			} while (sc != c);
>> -			if (len > slen)
>> -				return (NULL);
>> -		} while (strncmp(s, find, len) != 0);
>> -		s--;
>> -	}
>> -	return ((char *)s);
>> +  if (needle_len < haystack_len || !needle[needle_len]) {
>> +    char *x = memmem(haystack, haystack_len, needle, needle_len);
>> +    if (x && !memchr(haystack, 0, x - haystack))
>> +      return x;
>> +  }
>> +  return NULL;
>>   }
>> -- 
>> 2.7.4
> Input needed:
>
> Given this is a complete reimplementation not based on FreeBSD code at
> all, and given the code has been contributed by Eric, shouldn't we
> remove the entire copyright header and move this under newlib default
> licensing?
Including remove the "__FBSDID("$FreeBSD: head/lib/libc/string/strnstr.c 
251069 2013-05-28 20:57:40Z emaste $");", right?
>
> Thanks,
> Corinna
>

Re: [PATCH] Modify strnstr.c.

[Corinna Vinschen]

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

On Aug 28 18:50, Sichen Zhao wrote:
> > On Aug 26 21:28, Sichen Zhao wrote:
> > > ---
> > >   newlib/libc/string/strnstr.c | 24 ++++++++----------------
> > >   1 file changed, 8 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/newlib/libc/string/strnstr.c b/newlib/libc/string/strnstr.c
> > > index da5e5bd..7c87bbd 100644
> > > --- a/newlib/libc/string/strnstr.c
> > > +++ b/newlib/libc/string/strnstr.c
> > > @@ -44,22 +44,14 @@ __FBSDID("$FreeBSD: head/lib/libc/string/strnstr.c 251069 2013-05-28 20:57:40Z e
> > >    * first slen characters of s.
> > >    */
> > >   char *
> > > -strnstr(const char *s, const char *find, size_t slen)
> > > +strnstr(const char *haystack, const char *needle, size_t haystack_len)
> > >   {
> > > -	char c, sc;
> > > -	size_t len;
> > > +  size_t needle_len = strnlen(needle, haystack_len);
> > > -	if ((c = *find++) != '\0') {
> > > -		len = strlen(find);
> > > -		do {
> > > -			do {
> > > -				if (slen-- < 1 || (sc = *s++) == '\0')
> > > -					return (NULL);
> > > -			} while (sc != c);
> > > -			if (len > slen)
> > > -				return (NULL);
> > > -		} while (strncmp(s, find, len) != 0);
> > > -		s--;
> > > -	}
> > > -	return ((char *)s);
> > > +  if (needle_len < haystack_len || !needle[needle_len]) {
> > > +    char *x = memmem(haystack, haystack_len, needle, needle_len);
> > > +    if (x && !memchr(haystack, 0, x - haystack))
> > > +      return x;
> > > +  }
> > > +  return NULL;
> > >   }
> > > -- 
> > > 2.7.4
> > Input needed:
> > 
> > Given this is a complete reimplementation not based on FreeBSD code at
> > all, and given the code has been contributed by Eric, shouldn't we
> > remove the entire copyright header and move this under newlib default
> > licensing?
> Including remove the "__FBSDID("$FreeBSD: head/lib/libc/string/strnstr.c
> 251069 2013-05-28 20:57:40Z emaste $");", right?

Yes, that would make sense in that case I think.  But wait until
we have some input from Eric at least.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

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

Re: [PATCH] Modify strnstr.c.

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 924 bytes --]

On 08/28/2017 03:45 AM, Corinna Vinschen wrote:
> On Aug 26 21:28, Sichen Zhao wrote:
>> ---
>>  newlib/libc/string/strnstr.c | 24 ++++++++----------------
>>  1 file changed, 8 insertions(+), 16 deletions(-)
>>

> 
> Input needed:
> 
> Given this is a complete reimplementation not based on FreeBSD code at
> all, and given the code has been contributed by Eric, shouldn't we
> remove the entire copyright header and move this under newlib default
> licensing?

Seems reasonable to me (we aren't really copying the FreeBSD
implementation, just the interface). Default newlib licensing is fine;
it's also okay if you list yourself as author instead of me (I didn't
test my code, only suggested it), although if you want to put authorship
on my shoulders that's also okay.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [PATCH] Modify strnstr.c.

[Sichen Zhao]

> On 08/28/2017 03:45 AM, Corinna Vinschen wrote:
>> On Aug 26 21:28, Sichen Zhao wrote:
>>> ---
>>>   newlib/libc/string/strnstr.c | 24 ++++++++----------------
>>>   1 file changed, 8 insertions(+), 16 deletions(-)
>>>
>> Input needed:
>>
>> Given this is a complete reimplementation not based on FreeBSD code at
>> all, and given the code has been contributed by Eric, shouldn't we
>> remove the entire copyright header and move this under newlib default
>> licensing?
> Seems reasonable to me (we aren't really copying the FreeBSD
> implementation, just the interface). Default newlib licensing is fine;
> it's also okay if you list yourself as author instead of me (I didn't
> test my code, only suggested it), although if you want to put authorship
> on my shoulders that's also okay.
>
Ok, the license of newlib. i copy it from other c file in newlib, like:
/* Copyright 2017, Red Hat Inc. */
Is that ok?

Sichen

Re: [PATCH] Modify strnstr.c.

[Corinna Vinschen]

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

On Aug 28 23:14, Sichen Zhao wrote:
> > On 08/28/2017 03:45 AM, Corinna Vinschen wrote:
> > > On Aug 26 21:28, Sichen Zhao wrote:
> > > > ---
> > > >   newlib/libc/string/strnstr.c | 24 ++++++++----------------
> > > >   1 file changed, 8 insertions(+), 16 deletions(-)
> > > > 
> > > Input needed:
> > > 
> > > Given this is a complete reimplementation not based on FreeBSD code at
> > > all, and given the code has been contributed by Eric, shouldn't we
> > > remove the entire copyright header and move this under newlib default
> > > licensing?
> > Seems reasonable to me (we aren't really copying the FreeBSD
> > implementation, just the interface). Default newlib licensing is fine;
> > it's also okay if you list yourself as author instead of me (I didn't
> > test my code, only suggested it), although if you want to put authorship
> > on my shoulders that's also okay.
> > 
> Ok, the license of newlib. i copy it from other c file in newlib, like:
> /* Copyright 2017, Red Hat Inc. */
> Is that ok?

Just remove the FreeBSD copyright and don't add another one.  Files
without copyright default to newlib copyright anyway.

What would be pretty nifty, though: Can you add some man page entry,
along the lines of, say, libc/string/memmem.c?  This also requires a
tiny patch to libc/string/strings.tex.  This would not be a requirement
for what we're discussing here, just an idea for a followup patch.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

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