[ovs-dev] [patch_v9 1/6] string: Implement strcasestr substitute.

Sat Aug 5 03:09:19 UTC 2017

-----Original Message-----
From: <ovs-dev-bounces at openvswitch.org> on behalf of Ben Pfaff <blp at ovn.org>
Date: Friday, August 4, 2017 at 2:11 PM
To: Darrell Ball <dlu998 at gmail.com>
Cc: "dev at openvswitch.org" <dev at openvswitch.org>
Subject: Re: [ovs-dev] [patch_v9 1/6] string: Implement strcasestr	substitute.

    On Thu, Aug 03, 2017 at 09:34:41PM -0700, Darrell Ball wrote:
    > strcasestr is not defined for Windows, so implement a version
    > that could be used on Windows. This is needed for an upcoming
    > patch.
    > 
    > Signed-off-by: Darrell Ball <dlu998 at gmail.com>

    Thanks!

    I don't understand why this uses a macro to change the name of
    strcasestr to strcasestr_s.  I don't know of a benefit to that.  Maybe
    it follows the pattern in string.h.in that substitutes strerror_s for
    strerror_r and strtok_s to strtok_r, but those cases are because the
    Windows C library has functions named strerror_s and strtok_s that do
    what OVS wants.  The Windows C library does not (as far as I know) have
    a function strcasestr_s.

    The header should provide a prototype for the function.  It doesn't do
    clients any good to provide the prototype in the .c file.

Well, there is an explanation for everything – although it may not be a good one : ) ;
my intention was to provide a substitute with a separate name and use it for
Windows presently (with the remapping of strcasestr to strcasestr_s) but in the most
limited scope of source file usage.
However, I agree it is better just to provide a Windows version outright.

    The implementation should follow POSIX in that searching for an empty
    substring should always succeed; otherwise, eventually we will end up
    with confusion.  See:
    http://pubs.opengroup.org/onlinepubs/9699919799/functions/strstr.html

That was part of the reason for V9 - to support empty substr find success.
The V9 patch does handle the empty substring success case.
However, if both str and substr are empty, it should still be ‘success’; your version
does that; mine version returns NULL, which is non-compliant. 

    My understanding is that it is not a good idea, legally, to replace
    non-contiguous ranges of years by a range in a copyright notice.

ohh; thanks

    Here is my suggestion.  What do you think?

Nice
I was wondering if the below incremental looked ok to you ?
I like the two do/while loops for consistency, affirmative character matching and the explicit else ? 
It seems more definitional, but it might be slower and is not as compact, but either way is fine with me ?

+#ifdef _WIN32
    +char *
    +strcasestr(const char *str, const char *substr)
    +{
    +    size_t i = 0;
    +    do {
    +        do {
    +            if (!substr[i]) {
    +                return CONST_CAST(char *, str);
    +            } else if (tolower(substr[i]) == tolower(str[i])) {
    +                i++;
    +            } else {
    +                i = 0;
    +                break;
    +            }
    +        } while (1);
    +    } while (*str++);
    +    return NULL;
    +}
    +#endif

    Thanks,

    Ben.

    --8<--------------------------cut here-------------------------->8--

    From: Darrell Ball <dlu998 at gmail.com>
    Date: Thu, 3 Aug 2017 21:34:41 -0700
    Subject: [PATCH] string: Implement strcasestr substitute.

    strcasestr is not defined for Windows, so implement a version
    that could be used on Windows. This is needed for an upcoming
    patch.

    Signed-off-by: Darrell Ball <dlu998 at gmail.com>
    Signed-off-by: Ben Pfaff <blp at ovn.org>
    ---
     lib/string.c    | 23 +++++++++++++++++++++--
     lib/string.h.in |  4 +++-
     2 files changed, 24 insertions(+), 3 deletions(-)

    diff --git a/lib/string.c b/lib/string.c
    index 082359d858d8..a9ceabe47e9f 100644
    --- a/lib/string.c
    +++ b/lib/string.c
    @@ -1,5 +1,5 @@
     /*
    - * Copyright (c) 2009, 2011 Nicira, Inc.
    + * Copyright (c) 2009, 2011, 2017 Nicira, Inc.
      *
      * Licensed under the Apache License, Version 2.0 (the "License");
      * you may not use this file except in compliance with the License.
    @@ -15,9 +15,11 @@
      */

     #include <config.h>
    -
    +#include <ctype.h>
     #include <string.h>

    +#include "util.h"
    +
     #ifndef HAVE_STRNLEN
     size_t
     strnlen(const char *s, size_t maxlen)
    @@ -26,3 +28,20 @@ strnlen(const char *s, size_t maxlen)
         return end ? end - s : maxlen;
     }
     #endif
    +
    +#ifdef _WIN32
    +char *
    +strcasestr(const char *str, const char *substr)
    +{
    +    do {
    +        for (size_t i = 0; ; i++) {
    +            if (!substr[i]) {
    +                return CONST_CAST(char *, str);
    +            } else if (tolower(substr[i]) != tolower(str[i])) {
    +                break;
    +            }
    +        }
    +    } while (*str++);
    +    return NULL;
    +}
    +#endif
    diff --git a/lib/string.h.in b/lib/string.h.in
    index bbdaeb43e612..59998aa36fc4 100644
    --- a/lib/string.h.in
    +++ b/lib/string.h.in
    @@ -1,5 +1,5 @@
     /*
    - * Copyright (c) 2009, 2011, 2013 Nicira, Inc.
    + * Copyright (c) 2009-2017 Nicira, Inc.
      *
      * Licensed under the Apache License, Version 2.0 (the "License");
      * you may not use this file except in compliance with the License.
    @@ -36,6 +36,8 @@
     #define strcasecmp _stricmp
     #define strncasecmp _strnicmp
     #define strerror_r(errnum, buf, buflen) strerror_s(buf, buflen, errnum)
    +
    +char *strcasestr(const char *, const char *);
     #endif

     #ifndef HAVE_STRNLEN
    -- 
    2.10.2

    _______________________________________________
    dev mailing list
    dev at openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=CZi0Wy8JmOwf9_GckN8m8V6G9AEy6TgqHHE72EXjSEY&s=de6fTgs3jEuh_TRjEqi7_u-qre_oCQrb7aTlmrK-oHo&e= 