* [PATCH 0/3] string: further minor fixes
@ 2019-01-07 9:30 Jan Beulich
2019-01-07 9:39 ` [PATCH 1/3] string: avoid undefined behavior in strrchr() Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Jan Beulich @ 2019-01-07 9:30 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
Found while reviewing Andrew's str{,n}cmp() fix.
1: avoid undefined behavior in strrchr()
2: remove memscan()
3: fix type use in strstr()
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] string: avoid undefined behavior in strrchr()
2019-01-07 9:30 [PATCH 0/3] string: further minor fixes Jan Beulich
@ 2019-01-07 9:39 ` Jan Beulich
2019-01-07 9:49 ` Juergen Gross
2019-01-07 9:39 ` [PATCH 2/3] string: remove memscan() Jan Beulich
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2019-01-07 9:39 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
The pre-decrement would not only cause misbehavior when wrapping (benign
because there shouldn't be any NULL pointers passed in), but may also
create a pointer pointing outside the object that the passed in pointer
points to (it won't be de-referenced though). Use post-decrement (and >
instead of >= ) instead.
Take the opportunity and also
- convert bogus space (partly 7 of them) indentation to Linux style tab
one,
- add two blank lines.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -174,12 +174,13 @@ char *(strchr)(const char *s, int c)
*/
char *(strrchr)(const char *s, int c)
{
- const char *p = s + strlen(s);
- do {
- if (*p == (char)c)
- return (char *)p;
- } while (--p >= s);
- return NULL;
+ const char *p = s + strlen(s);
+
+ for (; *p != (char)c; --p)
+ if (p == s)
+ return NULL;
+
+ return (char *)p;
}
#endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] string: remove memscan()
2019-01-07 9:30 [PATCH 0/3] string: further minor fixes Jan Beulich
2019-01-07 9:39 ` [PATCH 1/3] string: avoid undefined behavior in strrchr() Jan Beulich
@ 2019-01-07 9:39 ` Jan Beulich
2019-01-07 9:50 ` Juergen Gross
2019-01-07 9:40 ` [PATCH 3/3] string: fix type use in strstr() Jan Beulich
2019-03-12 13:32 ` Ping: [PATCH 0/3] string: further minor fixes Jan Beulich
3 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2019-01-07 9:39 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
It has no users, so rather than fixing its use of types (first and
foremost c would need to be cast to unsigned char in the comparison
expression) drop it altogether. memchr() ought to be fine for all
purposes.
Take the opportunity and also do some stylistic adjustments to its
surviving sibling function memchr().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -380,30 +380,6 @@ int (memcmp)(const void *cs, const void
}
#endif
-#ifndef __HAVE_ARCH_MEMSCAN
-/**
- * memscan - Find a character in an area of memory.
- * @addr: The memory area
- * @c: The byte to search for
- * @size: The size of the area.
- *
- * returns the address of the first occurrence of @c, or 1 byte past
- * the area if @c is not found
- */
-void * memscan(void * addr, int c, size_t size)
-{
- unsigned char * p = (unsigned char *) addr;
-
- while (size) {
- if (*p == c)
- return (void *) p;
- p++;
- size--;
- }
- return (void *) p;
-}
-#endif
-
#ifndef __HAVE_ARCH_STRSTR
/**
* strstr - Find the first substring in a %NUL terminated string
@@ -441,14 +417,13 @@ char *(strstr)(const char *s1, const cha
void *(memchr)(const void *s, int c, size_t n)
{
const unsigned char *p = s;
- while (n-- != 0) {
- if ((unsigned char)c == *p++) {
- return (void *)(p-1);
- }
- }
+
+ while (n--)
+ if ((unsigned char)c == *p++)
+ return (void *)(p - 1);
+
return NULL;
}
-
#endif
/*
--- a/xen/include/xen/string.h
+++ b/xen/include/xen/string.h
@@ -96,10 +96,6 @@ void *memmove(void *, const void *, size
#define memmove(d, s, n) __builtin_memmove(d, s, n)
#endif
-#ifndef __HAVE_ARCH_MEMSCAN
-void *memscan(void *, int, size_t);
-#endif
-
#ifndef __HAVE_ARCH_MEMCMP
int memcmp(const void *, const void *, size_t);
#define memcmp(s1, s2, n) __builtin_memcmp(s1, s2, n)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] string: fix type use in strstr()
2019-01-07 9:30 [PATCH 0/3] string: further minor fixes Jan Beulich
2019-01-07 9:39 ` [PATCH 1/3] string: avoid undefined behavior in strrchr() Jan Beulich
2019-01-07 9:39 ` [PATCH 2/3] string: remove memscan() Jan Beulich
@ 2019-01-07 9:40 ` Jan Beulich
2019-01-07 9:53 ` Juergen Gross
2019-03-12 13:32 ` Ping: [PATCH 0/3] string: further minor fixes Jan Beulich
3 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2019-01-07 9:40 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
Using plain int for string lengths, while okay for all practical
purposes, is undesirable in a generic library function.
Take the opportunity and also move the function from being in the middle
of mem*() ones to the set of str*() ones, convert its loop from while()
to for(), and correct style.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/string.c
+++ b/xen/common/string.c
@@ -290,6 +290,27 @@ char * strsep(char **s, const char *ct)
}
#endif
+#ifndef __HAVE_ARCH_STRSTR
+/**
+ * strstr - Find the first substring in a %NUL terminated string
+ * @s1: The string to be searched
+ * @s2: The string to search for
+ */
+char *(strstr)(const char *s1, const char *s2)
+{
+ size_t l1, l2 = strlen(s2);
+
+ if (!l2)
+ return (char *)s1;
+
+ for (l1 = strlen(s1); l1 >= l2; --l1, ++s1)
+ if (!memcmp(s1, s2, l2))
+ return (char *)s1;
+
+ return NULL;
+}
+#endif
+
#ifndef __HAVE_ARCH_MEMSET
/**
* memset - Fill a region of memory with the given value
@@ -380,30 +401,6 @@ int (memcmp)(const void *cs, const void
}
#endif
-#ifndef __HAVE_ARCH_STRSTR
-/**
- * strstr - Find the first substring in a %NUL terminated string
- * @s1: The string to be searched
- * @s2: The string to search for
- */
-char *(strstr)(const char *s1, const char *s2)
-{
- int l1, l2;
-
- l2 = strlen(s2);
- if (!l2)
- return (char *) s1;
- l1 = strlen(s1);
- while (l1 >= l2) {
- l1--;
- if (!memcmp(s1,s2,l2))
- return (char *) s1;
- s1++;
- }
- return NULL;
-}
-#endif
-
#ifndef __HAVE_ARCH_MEMCHR
/**
* memchr - Find a character in an area of memory.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] string: avoid undefined behavior in strrchr()
2019-01-07 9:39 ` [PATCH 1/3] string: avoid undefined behavior in strrchr() Jan Beulich
@ 2019-01-07 9:49 ` Juergen Gross
2019-01-07 10:04 ` Jan Beulich
0 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2019-01-07 9:49 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
On 07/01/2019 10:39, Jan Beulich wrote:
> The pre-decrement would not only cause misbehavior when wrapping (benign
> because there shouldn't be any NULL pointers passed in), but may also
> create a pointer pointing outside the object that the passed in pointer
> points to (it won't be de-referenced though). Use post-decrement (and >
> instead of >= ) instead.
This commit message doesn't match the patch. There is no post-decrement
and '>' involved.
With that fixed you can add:
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
>
> Take the opportunity and also
> - convert bogus space (partly 7 of them) indentation to Linux style tab
> one,
> - add two blank lines.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/string.c
> +++ b/xen/common/string.c
> @@ -174,12 +174,13 @@ char *(strchr)(const char *s, int c)
> */
> char *(strrchr)(const char *s, int c)
> {
> - const char *p = s + strlen(s);
> - do {
> - if (*p == (char)c)
> - return (char *)p;
> - } while (--p >= s);
> - return NULL;
> + const char *p = s + strlen(s);
> +
> + for (; *p != (char)c; --p)
> + if (p == s)
> + return NULL;
> +
> + return (char *)p;
> }
> #endif
>
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] string: remove memscan()
2019-01-07 9:39 ` [PATCH 2/3] string: remove memscan() Jan Beulich
@ 2019-01-07 9:50 ` Juergen Gross
0 siblings, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2019-01-07 9:50 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
On 07/01/2019 10:39, Jan Beulich wrote:
> It has no users, so rather than fixing its use of types (first and
> foremost c would need to be cast to unsigned char in the comparison
> expression) drop it altogether. memchr() ought to be fine for all
> purposes.
>
> Take the opportunity and also do some stylistic adjustments to its
> surviving sibling function memchr().
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/string.c
> +++ b/xen/common/string.c
> @@ -380,30 +380,6 @@ int (memcmp)(const void *cs, const void
> }
> #endif
>
> -#ifndef __HAVE_ARCH_MEMSCAN
> -/**
> - * memscan - Find a character in an area of memory.
> - * @addr: The memory area
> - * @c: The byte to search for
> - * @size: The size of the area.
> - *
> - * returns the address of the first occurrence of @c, or 1 byte past
> - * the area if @c is not found
> - */
> -void * memscan(void * addr, int c, size_t size)
> -{
> - unsigned char * p = (unsigned char *) addr;
> -
> - while (size) {
> - if (*p == c)
> - return (void *) p;
> - p++;
> - size--;
> - }
> - return (void *) p;
> -}
> -#endif
> -
> #ifndef __HAVE_ARCH_STRSTR
> /**
> * strstr - Find the first substring in a %NUL terminated string
> @@ -441,14 +417,13 @@ char *(strstr)(const char *s1, const cha
> void *(memchr)(const void *s, int c, size_t n)
> {
> const unsigned char *p = s;
> - while (n-- != 0) {
> - if ((unsigned char)c == *p++) {
> - return (void *)(p-1);
> - }
> - }
> +
> + while (n--)
> + if ((unsigned char)c == *p++)
White space damage.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] string: fix type use in strstr()
2019-01-07 9:40 ` [PATCH 3/3] string: fix type use in strstr() Jan Beulich
@ 2019-01-07 9:53 ` Juergen Gross
0 siblings, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2019-01-07 9:53 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall
On 07/01/2019 10:40, Jan Beulich wrote:
> Using plain int for string lengths, while okay for all practical
> purposes, is undesirable in a generic library function.
>
> Take the opportunity and also move the function from being in the middle
> of mem*() ones to the set of str*() ones, convert its loop from while()
> to for(), and correct style.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] string: avoid undefined behavior in strrchr()
2019-01-07 9:49 ` Juergen Gross
@ 2019-01-07 10:04 ` Jan Beulich
0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2019-01-07 10:04 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, xen-devel
>>> On 07.01.19 at 10:49, <jgross@suse.com> wrote:
> On 07/01/2019 10:39, Jan Beulich wrote:
>> The pre-decrement would not only cause misbehavior when wrapping (benign
>> because there shouldn't be any NULL pointers passed in), but may also
>> create a pointer pointing outside the object that the passed in pointer
>> points to (it won't be de-referenced though). Use post-decrement (and >
>> instead of >= ) instead.
>
> This commit message doesn't match the patch. There is no post-decrement
> and '>' involved.
Oh, indeed, that was stale from a first attempt, until I realized that
the post-decrement would make things only marginally better. I've
simply dropped that sentence.
> With that fixed you can add:
>
> Reviewed-by: Juergen Gross <jgross@suse.com>
Thanks, Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Ping: [PATCH 0/3] string: further minor fixes
2019-01-07 9:30 [PATCH 0/3] string: further minor fixes Jan Beulich
` (2 preceding siblings ...)
2019-01-07 9:40 ` [PATCH 3/3] string: fix type use in strstr() Jan Beulich
@ 2019-03-12 13:32 ` Jan Beulich
2019-03-12 15:50 ` Andrew Cooper
3 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2019-03-12 13:32 UTC (permalink / raw)
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Julien Grall, xen-devel
>>> On 07.01.19 at 10:30, wrote:
> Found while reviewing Andrew's str{,n}cmp() fix.
>
> 1: avoid undefined behavior in strrchr()
> 2: remove memscan()
I've taken care of the formatting issue pointed out by Jürgen here,
but it didn't seem worthwhile to send v2 just for this.
Jan
> 3: fix type use in strstr()
>
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Ping: [PATCH 0/3] string: further minor fixes
2019-03-12 13:32 ` Ping: [PATCH 0/3] string: further minor fixes Jan Beulich
@ 2019-03-12 15:50 ` Andrew Cooper
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Cooper @ 2019-03-12 15:50 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Ian Jackson, Julien Grall, xen-devel
On 12/03/2019 13:32, Jan Beulich wrote:
>>>> On 07.01.19 at 10:30, wrote:
>> Found while reviewing Andrew's str{,n}cmp() fix.
>>
>> 1: avoid undefined behavior in strrchr()
>> 2: remove memscan()
> I've taken care of the formatting issue pointed out by Jürgen here,
> but it didn't seem worthwhile to send v2 just for this.
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-03-12 16:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 9:30 [PATCH 0/3] string: further minor fixes Jan Beulich
2019-01-07 9:39 ` [PATCH 1/3] string: avoid undefined behavior in strrchr() Jan Beulich
2019-01-07 9:49 ` Juergen Gross
2019-01-07 10:04 ` Jan Beulich
2019-01-07 9:39 ` [PATCH 2/3] string: remove memscan() Jan Beulich
2019-01-07 9:50 ` Juergen Gross
2019-01-07 9:40 ` [PATCH 3/3] string: fix type use in strstr() Jan Beulich
2019-01-07 9:53 ` Juergen Gross
2019-03-12 13:32 ` Ping: [PATCH 0/3] string: further minor fixes Jan Beulich
2019-03-12 15:50 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.