Re: [PATCH] block/curl: use strlen instead of strchr

[Michael Tokarev]

On 6/29/24 09:20, Michael Tokarev wrote:
> On 6/28/24 08:49, Vladimir Sementsov-Ogievskiy wrote:
> > We already know where colon is, so no reason to search for it. Also,
> > avoid a code, which looks like we forget to check return value of
> > strchr() to NULL.
> >
> > Suggested-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > ---
> >
> > This replaces my patch
> >    [PATCH] block/curl: explicitly assert that strchr returns non-NULL value
> > Supersedes: <20240627153059.589070-1-vsementsov@yandex-team.ru>
> >
> >   block/curl.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/curl.c b/block/curl.c
> > index 419f7c89ef..d03bfe4817 100644
> > --- a/block/curl.c
> > +++ b/block/curl.c
> > @@ -219,7 +219,7 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t 
> > nmemb, void *opaque)
> >           && g_ascii_strncasecmp(header, accept_ranges,
> >                                  strlen(accept_ranges)) == 0) {
> > -        char *p = strchr(header, ':') + 1;
> > +        char *p = header + strlen(accept_ranges);
> >           /* Skip whitespace between the header name and value. */
> >           while (p < end && *p && g_ascii_isspace(*p)) {
>
> Heck.  All these strlen()s look ugly, especially in the
> loop iterations..  To my taste anyway.
>
> How about this instead:

Or even this:

diff --git a/block/curl.c b/block/curl.c
index 419f7c89ef..982597ea7f 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -210,37 +210,28 @@ static size_t curl_header_cb(void *ptr, size_t size, 
size_t nmemb, void *opaque)
 {
     BDRVCURLState *s = opaque;
     size_t realsize = size * nmemb;
-    const char *header = (char *)ptr;
-    const char *end = header + realsize;
-    const char *accept_ranges = "accept-ranges:";
-    const char *bytes = "bytes";
-
-    if (realsize >= strlen(accept_ranges)
-        && g_ascii_strncasecmp(header, accept_ranges,
-                               strlen(accept_ranges)) == 0) {
-
-        char *p = strchr(header, ':') + 1;
-
-        /* Skip whitespace between the header name and value. */
-        while (p < end && *p && g_ascii_isspace(*p)) {
-            p++;
-        }
-
-        if (end - p >= strlen(bytes)
-            && strncmp(p, bytes, strlen(bytes)) == 0) {
-
-            /* Check that there is nothing but whitespace after the value. */
-            p += strlen(bytes);
-            while (p < end && *p && g_ascii_isspace(*p)) {
-                p++;
-            }
-
-            if (p == end || !*p) {
-                s->accept_range = true;
+    const char *p = (char *)ptr;
+    const char *end = p + realsize;
+    const char *t = "accept-ranges : bytes ";
+
+    while (p < end && *t) {
+        if (*t == ' ') {
+            if (g_ascii_isspace(*p)) {
+                ++p;
+            } else {
+                ++t;
             }
+        } else if (*t == g_ascii_tolower(*p)) {
+            ++p, ++t;
+        } else {
+            break;
         }
     }

+    if (p == end || !*p) {
+        s->accept_range = true;
+    }
+
     return realsize;
 }

Whole thing:

static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
{
    BDRVCURLState *s = opaque;
    size_t realsize = size * nmemb;
    const char *p = (char *)ptr;
    const char *end = p + realsize;
    const char *t = "accept-ranges : bytes ";

    while (p < end && *t) {
        if (*t == ' ') {
            if (g_ascii_isspace(*p)) {
                ++p;
            } else {
                ++t;
            }
        } else if (*t == g_ascii_tolower(*p)) {
            ++p, ++t;
        } else {
            break;
        }
    }

    if (p == end || !*p) {
        s->accept_range = true;
    }

    return realsize;
}

/mjt

--
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 
ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 
8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt