5
\$\begingroup\$

Here's what I came up with. What improvements might I make? All I want you to look at is the strncmpci() function--nothing else for improvements. Note that I want readable and maintainable code, NOT optimized code which is impossible to read.

Goals:

  1. bug-free (does what it says it does, and does it well)
  2. handles potential edge and corner cases and other pitfalls
  3. avoids undefined behavior
  4. readable
  5. fast
  6. (Added late, but intended this all along) runs properly and well on 8-bit mcus as well as on modern 64-bit computers and operating systems; the goal is to have one version of this function I can use anywhere I need it instead of having to think about it and rewrite it each time.
  7. low-memory footprint in RAM (ie: can easily run on an 8-bit microcontroller, such as Arduino)
  8. low-memory footprint in program space (ie: can easily be stored on an 8-bit microcontroller, such as Arduino)
  9. doesn't need to handle UTF-8 (but if you want to provide an implementation that does, that would be awesome; if you do provide a UTF-8 implementation it does not have to be able to run on a microcontroller)
  10. I wrote this in C, but if C++ provides some handy libraries that makes this easier or UTF-8 possible you may present them as an alternative

From my answer here: Stack Overflow: Case Insensitive String Comparison in C

This is a direct drop-in replacement for strncmp(), and has been tested with numerous test cases, as shown below.

It is identical to strncmp() except:

  1. It is case-insensitive.
  2. The behavior is NOT undefined (it is well-defined) if either string is a null ptr. Regular strncmp() has undefined behavior if either string is a null ptr (see: https://en.cppreference.com/w/cpp/string/byte/strncmp).
  3. It returns INT_MIN as a special sentinel error value if either input string is a NULL ptr.

LIMITATIONS: Note that this code works on the original 7-bit ASCII character set only (decimal values 0 to 127, inclusive), NOT on unicode characters, such as unicode character encodings UTF-8 (the most popular), UTF-16, and UTF-32.

Here is the code only (no comments):

int strncmpci(const char * str1, const char * str2, size_t num)
{
    int ret_code = 0;
    size_t chars_compared = 0;

    if (!str1 || !str2)
    {
        ret_code = INT_MIN;
        return ret_code;
    }

    while ((chars_compared < num) && (*str1 || *str2))
    {
        ret_code = tolower((int)(*str1)) - tolower((int)(*str2));
        if (ret_code != 0)
        {
            break;
        }
        chars_compared++;
        str1++;
        str2++;
    }

    return ret_code;
}

Fully-commented version:

/// \brief      Perform a case-insensitive string compare (`strncmp()` case-insensitive) to see
///             if two C-strings are equal.
/// \note       1. Identical to `strncmp()` except:
///               1. It is case-insensitive.
///               2. The behavior is NOT undefined (it is well-defined) if either string is a null
///               ptr. Regular `strncmp()` has undefined behavior if either string is a null ptr
///               (see: https://en.cppreference.com/w/cpp/string/byte/strncmp).
///               3. It returns `INT_MIN` as a special sentinel value for certain errors.
///             - Posted as an answer here: https://stackoverflow.com/a/55293507/4561887.
///               - Aided/inspired, in part, by `strcicmp()` here:
///                 https://stackoverflow.com/a/5820991/4561887.
/// \param[in]  str1        C string 1 to be compared.
/// \param[in]  str2        C string 2 to be compared.
/// \param[in]  num         max number of chars to compare
/// \return     A comparison code (identical to `strncmp()`, except with the addition
///             of `INT_MIN` as a special sentinel value):
///
///             INT_MIN (usually -2147483648 for int32_t integers)  Invalid arguments (one or both
///                      of the input strings is a NULL pointer).
///             <0       The first character that does not match has a lower value in str1 than
///                      in str2.
///              0       The contents of both strings are equal.
///             >0       The first character that does not match has a greater value in str1 than
///                      in str2.
int strncmpci(const char * str1, const char * str2, size_t num)
{
    int ret_code = 0;
    size_t chars_compared = 0;

    // Check for NULL pointers
    if (!str1 || !str2)
    {
        ret_code = INT_MIN;
        return ret_code;
    }

    // Continue doing case-insensitive comparisons, one-character-at-a-time, of `str1` to `str2`, so
    // long as 1st: we have not yet compared the requested number of chars, and 2nd: the next char
    // of at least *one* of the strings is not zero (the null terminator for a C-string), meaning
    // that string still has more characters in it.
    // Note: you MUST check `(chars_compared < num)` FIRST or else dereferencing (reading) `str1` or
    // `str2` via `*str1` and `*str2`, respectively, is undefined behavior if you are reading one or
    // both of these C-strings outside of their array bounds.
    while ((chars_compared < num) && (*str1 || *str2))
    {
        ret_code = tolower((int)(*str1)) - tolower((int)(*str2));
        if (ret_code != 0)
        {
            // The 2 chars just compared don't match
            break;
        }
        chars_compared++;
        str1++;
        str2++;
    }

    return ret_code;
}

Test code:

Download the entire sample code, with unit tests, from my eRCaGuy_hello_world repository here: "strncmpci.c":


// This file is part of eRCaGuy_hello_world: https://github.com/ElectricRCAircraftGuy/eRCaGuy_hello_world

/*
strncmpci.c

- A 'c'ase 'i'nsensitive version of `strncmp()`.
- See references below for more info., including documentation for `strncmp()`, as well as my
Stack Overflow answer where I present my `strncmpci()` function below.

Gabriel Staples
www.ElectricRCAircraftGuy.com
Written: 21 Mar. 2019
Updated: 21 Oct. 2020
- moved to this git repo; see `git log` history after that

To compile and run:

    gcc -Wall -Wextra -Werror -ggdb -std=c11 -o ./bin/tmp strncmpci.c && ./bin/tmp


References:
1. [my own answer] https://stackoverflow.com/questions/5820810/case-insensitive-string-comp-in-c/55293507#55293507
2. https://en.cppreference.com/w/cpp/string/byte/strncmp
3. http://www.cplusplus.com/reference/cstring/strncmp/

STATUS:
IT WORKS! ALL UNIT TESTS PASS!

*/

#include <assert.h>
#include <stdbool.h>
#include <ctype.h> // for `tolower()`
#include <limits.h> // for `INT_MIN`
#include <stdio.h>
#include <string.h>

// For ANSI color codes in a terminal, see my notes to self in my file here:
// https://github.com/ElectricRCAircraftGuy/eRCaGuy_dotfiles/blob/master/useful_scripts/git-diffn.sh
#define ANSI_COLOR_OFF "\033[m"
#define ANSI_COLOR_GRN "\033[32m"
#define ANSI_COLOR_RED "\033[31m"

typedef struct data_s
{
    int error_count;
} data_t;

// Data struct used to safely contain and pass around global data
data_t globals = {
    .error_count = 0,
};

// TODO: Make a version of this code which also works on Unicode's UTF-8 implementation (character
// encoding)! Add it to my answer here too: https://stackoverflow.com/a/55293507/4561887.

/// \brief      Perform a case-insensitive string compare (`strncmp()` case-insensitive) to see
///             if two C-strings are equal.
/// \note       1. Identical to `strncmp()` except:
///               1. It is case-insensitive.
///               2. The behavior is NOT undefined (it is well-defined) if either string is a null
///               ptr. Regular `strncmp()` has undefined behavior if either string is a null ptr
///               (see: https://en.cppreference.com/w/cpp/string/byte/strncmp).
///               3. It returns `INT_MIN` as a special sentinel value for certain errors.
///             - Posted as an answer here: https://stackoverflow.com/a/55293507/4561887.
///               - Aided/inspired, in part, by `strcicmp()` here:
///                 https://stackoverflow.com/a/5820991/4561887.
/// \param[in]  str1        C string 1 to be compared.
/// \param[in]  str2        C string 2 to be compared.
/// \param[in]  num         max number of chars to compare
/// \return     A comparison code (identical to `strncmp()`, except with the addition
///             of `INT_MIN` as a special sentinel value):
///
///             INT_MIN (usually -2147483648 for int32_t integers)  Invalid arguments (one or both
///                      of the input strings is a NULL pointer).
///             <0       The first character that does not match has a lower value in str1 than
///                      in str2.
///              0       The contents of both strings are equal.
///             >0       The first character that does not match has a greater value in str1 than
///                      in str2.
int strncmpci(const char * str1, const char * str2, size_t num)
{
    int ret_code = 0;
    size_t chars_compared = 0;

    // Check for NULL pointers
    if (!str1 || !str2)
    {
        ret_code = INT_MIN;
        return ret_code;
    }

    // Continue doing case-insensitive comparisons, one-character-at-a-time, of `str1` to `str2`,
    // as long as at least one of the strings still has more characters in it, and we have
    // not yet compared `num` chars.
    while ((*str1 || *str2) && (chars_compared < num))
    {
        ret_code = tolower((int)(*str1)) - tolower((int)(*str2));
        if (ret_code != 0)
        {
            // The 2 chars just compared don't match
            break;
        }
        chars_compared++;
        str1++;
        str2++;
    }

    return ret_code;
}

// VERSION **WITH** GOTO. This is perfectly valid and safe usage of `goto`, but some people
// may have a problem with it, and it's suuuuper easy to avoid in this simple example code,
// so let's remove it for the main version above but leave it for this version below.

/// \brief      Perform a case-insensitive string compare (`strncmp()` case-insensitive) to see
///             if two C-strings are equal.
/// \note       1. Identical to `strncmp()` except:
///               1. It is case-insensitive.
///               2. The behavior is NOT undefined (it is well-defined) if either string is a null
///               ptr. Regular `strncmp()` has undefined behavior if either string is a null ptr
///               (see: https://en.cppreference.com/w/cpp/string/byte/strncmp).
///               3. It returns `INT_MIN` as a special sentinel value for certain errors.
///             - Posted as an answer here: https://stackoverflow.com/a/55293507/4561887.
///               - Aided/inspired, in part, by `strcicmp()` here:
///                 https://stackoverflow.com/a/5820991/4561887.
/// \param[in]  str1        C string 1 to be compared.
/// \param[in]  str2        C string 2 to be compared.
/// \param[in]  num         max number of chars to compare
/// \return     A comparison code (identical to `strncmp()`, except with the addition
///             of `INT_MIN` as a special sentinel value):
///
///             INT_MIN (usually -2147483648 for int32_t integers)  Invalid arguments (one or both
///                      of the input strings is a NULL pointer).
///             <0       The first character that does not match has a lower value in str1 than
///                      in str2.
///              0       The contents of both strings are equal.
///             >0       The first character that does not match has a greater value in str1 than
///                      in str2.
int strncmpci2(const char * str1, const char * str2, size_t num)
{
    int ret_code = 0;
    size_t chars_compared = 0;

    // Check for NULL pointers
    if (!str1 || !str2)
    {
        ret_code = INT_MIN;
        goto done;
    }

    // Continue doing case-insensitive comparisons, one-character-at-a-time, of `str1` to `str2`,
    // as long as at least one of the strings still has more characters in it, and we have
    // not yet compared `num` chars.
    while ((*str1 || *str2) && (chars_compared < num))
    {
        ret_code = tolower((int)(*str1)) - tolower((int)(*str2));
        if (ret_code != 0)
        {
            // The 2 chars just compared don't match
            break;
        }
        chars_compared++;
        str1++;
        str2++;
    }

done:
    return ret_code;
}

// TODO: ADD IN Unit tests to test this function too! Ex: `EXPECT_EQUALS(strcicmp(str1, str2), 0);`

// /// \brief      Alternative approach to test and compare results from.
// /// \note       Copied directly from here:
// ///             https://stackoverflow.com/questions/5820810/case-insensitive-string-comp-in-c/5820991#5820991
// int strcicmp(char const *a, char const *b)
// {
//     for (;; a++, b++) {
//         int d = tolower((unsigned char)*a) - tolower((unsigned char)*b);
//         if (d != 0 || !*a)
//             return d;
//     }
// }

/// \brief      Wrapper around the below unit test function.
/// \details    Sample usage:
///                 EXPECT_EQUALS(strncmpci(str1, str2, n), 1);
///             Sample output:
///                 FAILED at line 173 in function main! strncmpci(str1, str2, n) != 1
///                   a: strncmpci(str1, str2, n) is 0
///                   b: 1 is 1
#define EXPECT_EQUALS(int_a, int_b) \
    do { \
        expect_equals(int_a, int_b, &globals.error_count, #int_a, #int_b, __LINE__, __func__); \
    } while (false)

/// \brief      Perform a simple unit test to see if int a == int b.
/// \param[in]      a            the first integer to compare
/// \param[in]      b            the second integer to compare
/// \param[in,out]  error_count  (Optional) a total error counter which will be incremented in the
///                     event a != b. Pass in NULL to not use.
/// \param[in]      a_str        (Optional) a string to print to represent what was passed in for
///                              `a`. Pass in NULL to not use.
/// \param[in]      b_str        (Optional) a string to print to represent what was passed in for
///                              `b`. Pass in NULL to not use.
/// \param[in]      line         The line number of the call site; pass in `__LINE__`. See:
///                              https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html.
/// \param[in]      func         The function of the call site; pass in `__func__`. See:
///                              https://gcc.gnu.org/onlinedocs/gcc/Function-Names.html.
/// \return     true if a == b, and false otherwise
bool expect_equals(int a, int b, int * error_count, char * a_str, char * b_str, int line,
    const char * func)
{
    if (a == b)
    {
        return true;
    }

    if (error_count != NULL)
    {
        (*error_count)++;
    }

    if (a_str == NULL || b_str == NULL)
    {
        printf("FAILED at line %i in function %s! a != b\n"
               "  a is %i\n"
               "  b is %i\n\n",
               line, func, a, b);
    }
    else
    {
        // both a_str and b_str are NOT null ptrs
        printf("FAILED at line %i in function %s! %s != %s\n"
               "  a: %s is %i\n"
               "  b: %s is %i\n\n",
               line, func, a_str, b_str, a_str, a, b_str, b);
    }

    return false;
}

int main()
{
    printf("-----------------------\n"
           "String Comparison Tests\n"
           "-----------------------\n\n");

    int num_failures_expected = 0;

    printf("INTENTIONAL UNIT TEST FAILURE to show what a unit test failure looks like!\n");
    EXPECT_EQUALS(strncmpci("hey", "HEY", 3), 'h' - 'H');
    num_failures_expected++;
    printf("------ beginning ------\n\n");


    const char * str1;
    const char * str2;
    size_t n;

    // NULL ptr checks
    EXPECT_EQUALS(strncmpci(NULL, "", 0), INT_MIN);
    EXPECT_EQUALS(strncmpci("", NULL, 0), INT_MIN);
    EXPECT_EQUALS(strncmpci(NULL, NULL, 0), INT_MIN);
    EXPECT_EQUALS(strncmpci(NULL, "", 10), INT_MIN);
    EXPECT_EQUALS(strncmpci("", NULL, 10), INT_MIN);
    EXPECT_EQUALS(strncmpci(NULL, NULL, 10), INT_MIN);

    EXPECT_EQUALS(strncmpci("", "", 0), 0);
    EXPECT_EQUALS(strncmp("", "", 0), 0);

    str1 = "";
    str2 = "";
    n = 0;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 0);

    str1 = "hey";
    str2 = "HEY";
    n = 0;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 0);

    str1 = "hey";
    str2 = "HEY";
    n = 3;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 'h' - 'H');

    str1 = "heY";
    str2 = "HeY";
    n = 3;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 'h' - 'H');

    str1 = "hey";
    str2 = "HEdY";
    n = 3;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 'y' - 'd');
    EXPECT_EQUALS(strncmp(str1, str2, n), 'h' - 'H');

    str1 = "heY";
    str2 = "hEYd";
    n = 3;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 'e' - 'E');

    str1 = "heY";
    str2 = "heyd";
    n = 6;
    EXPECT_EQUALS(strncmpci(str1, str2, n), -'d');
    EXPECT_EQUALS(strncmp(str1, str2, n), 'Y' - 'y');

    str1 = "hey";
    str2 = "hey";
    n = 6;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 0);

    str1 = "hey";
    str2 = "heyd";
    n = 6;
    EXPECT_EQUALS(strncmpci(str1, str2, n), -'d');
    EXPECT_EQUALS(strncmp(str1, str2, n), -'d');

    str1 = "hey";
    str2 = "heyd";
    n = 3;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 0);

    str1 = "hEY";
    str2 = "heyYOU";
    n = 3;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 0);
    EXPECT_EQUALS(strncmp(str1, str2, n), 'E' - 'e');

    str1 = "hEY";
    str2 = "heyYOU";
    n = 10;
    EXPECT_EQUALS(strncmpci(str1, str2, n), -'y');
    EXPECT_EQUALS(strncmp(str1, str2, n), 'E' - 'e');

    str1 = "hEYHowAre";
    str2 = "heyYOU";
    n = 10;
    EXPECT_EQUALS(strncmpci(str1, str2, n), 'h' - 'y');
    EXPECT_EQUALS(strncmp(str1, str2, n), 'E' - 'e');

    EXPECT_EQUALS(strncmpci("nice to meet you.,;", "NICE TO MEET YOU.,;", 100), 0);
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "NICE TO MEET YOU.,;", 100), 'n' - 'N');
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "nice to meet you.,;", 100), 0);

    EXPECT_EQUALS(strncmpci("nice to meet you.,;", "NICE TO UEET YOU.,;", 100), 'm' - 'u');
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "nice to uEET YOU.,;", 100), 'm' - 'u');
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "nice to UEET YOU.,;", 100), 'm' - 'U');

    EXPECT_EQUALS(strncmpci("nice to meet you.,;", "NICE TO MEET YOU.,;", 5), 0);
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "NICE TO MEET YOU.,;", 5), 'n' - 'N');

    EXPECT_EQUALS(strncmpci("nice to meet you.,;", "NICE eo UEET YOU.,;", 5), 0);
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "nice eo uEET YOU.,;", 5), 0);

    EXPECT_EQUALS(strncmpci("nice to meet you.,;", "NICE eo UEET YOU.,;", 100), 't' - 'e');
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "nice eo uEET YOU.,;", 100), 't' - 'e');

    EXPECT_EQUALS(strncmpci("nice to meet you.,;", "nice-eo UEET YOU.,;", 5), ' ' - '-');
    EXPECT_EQUALS(strncmp(  "nice to meet you.,;", "nice-eo UEET YOU.,;", 5), ' ' - '-');


    if (globals.error_count == num_failures_expected)
    {
        printf(ANSI_COLOR_GRN "All unit tests passed!" ANSI_COLOR_OFF "\n");
    }
    else
    {
        printf(ANSI_COLOR_RED "FAILED UNIT TESTS! NUMBER OF UNEXPECTED FAILURES = %i"
            ANSI_COLOR_OFF "\n", globals.error_count - num_failures_expected);
    }

    assert(globals.error_count == num_failures_expected);
    return globals.error_count;
}

/*
Sample output:

    $ gcc -Wall -Wextra -Werror -ggdb -std=c11 -o ./bin/tmp strncmpci.c && ./bin/tmp
    -----------------------
    String Comparison Tests
    -----------------------

    INTENTIONAL UNIT TEST FAILURE to show what a unit test failure looks like!
    FAILED at line 191 in function main! strncmpci("hey", "HEY", 3) != 'h' - 'H'
      a: strncmpci("hey", "HEY", 3) is 0
      b: 'h' - 'H' is 32

    ------ beginning ------

    All unit tests passed!


*/

Sample output:

$ gcc -Wall -Wextra -Werror -ggdb -std=c11 -o ./bin/tmp strncmpci.c && ./bin/tmp
-----------------------
String Comparison Tests
-----------------------

INTENTIONAL UNIT TEST FAILURE to show what a unit test failure looks like!
FAILED at line 250 in function main! strncmpci("hey", "HEY", 3) != 'h' - 'H'
  a: strncmpci("hey", "HEY", 3) is 0
  b: 'h' - 'H' is 32

------ beginning ------

All unit tests passed!
\$\endgroup\$
3
  • 1
    \$\begingroup\$ nothing else for improvements is somewhat antithetical to this site. When code is posted, all feedback is on topic. \$\endgroup\$
    – Reinderien
    Jan 29, 2021 at 4:32
  • \$\begingroup\$ @Reinderien, yeah, that's fine. I don't mean people can't improve the other parts, I just mean the other parts are not the focus of my question so I'm not requesting people try to improve them. \$\endgroup\$ Jan 29, 2021 at 4:33
  • 1
    \$\begingroup\$ @Reinderien "all feedback is on topic" is not the issue as much as how relevant it is to the question and OP's goals. Code Review encourages focus. For OP to say here "look at is the strncmpci() function--nothing else for improvements." simply implies feedback on the other code is not the main concern - but there for (test) convivence. Sure we could comment about int main() vs. int main(void), but distracts from a good answer. OTOH pointing out a weakness in tests that did not expose a problem in strncmpci() certainly is good. \$\endgroup\$ Jan 29, 2021 at 13:30

2 Answers 2

6
\$\begingroup\$

What improvements might I make?

Compare with to lower or to upper?

int strncmpci() deserves documentation indicating its choice in the comments above int strncmpci(...).

Characters like '_', In ASCII, exist between the upper and lower case letters. Sorting text with "[]^_" and others come before or after A-Z depending on which is used.

Note that together with letters outside the ASCII range, letters may not have a one-to-one mapping between upper and lower case. This creates situations where 2 strings compare equal when folded to lower case but not when folded to upper case - and visa-versa. I suspect OP is indifferent to these cases, so will no go further.

No need to check both *str1, *str2

Testing for a mis-match and only one string's null character is sufficient - unless you are concerned that the lower case version of some other character is also the null character.

Questionable cast

I see little value to the (int) cast and see value in other approaches.

When char is signed, tolower((int)(*str1)) is UB when *str1 < 0 and not EOF.

C specifies

In all cases the argument is an int, the value of which shall be representable as an unsigned char or shall equal the value of the macro EOF. If the argument has any other value, the behavior is undefined. C17dr § 7.4.1 1

This is very simply to meet by using unsigned char *

int strncmpci(const char * str1, const char * str2, size_t num) {
  const unsigned char *ustr1 = (const unsigned char *)str1;
  const unsigned char *ustr2 = (const unsigned char *)str2;
  // Now instead use `ustr1, ustr2` for rest of code.

    ret_code = tolower(*ustr1) - tolower(*ustr2);

OP has "code works on the original 7-bit ASCII character set only". There is not need to not behave well with characters outside the ASCII range.

Further to match strncmp(), code should "For all functions in this subclause, each character shall be interpreted as if it had the type unsigned char".

Parameter order

C2X may have " ... the order of parameters in function declarations should be arranged such that the size of an array appears before the array. ...". So if not trying for backward compatibility, one could code:

//int strncmpci(const char * str1, const char * str2, size_t num)
int strncmpci(size_t num, const char * str1, const char * str2)

Sorting

The return value of INT_MIN for error cases makes using this function as part of a compare function a problem with qsort() as strncmpci(NULL, "foo"); returns INT_MIN, yet strncmpci( "foo", NULL); needs to return a positive number to meet the compare function requirements c17dr § 7.22.5.1 3.

The whole error checking for NULL arguments sounds/looks nice, but may have unintended consequences.

Candidate alternative:

int strncmpci(const char * str1, const char * str2, size_t num) {
  if (str1 == NULL || str2 == NULL) {
    errno = TBD;
    if (str1 == NULL) str1 = "";
    if (str2 == NULL) str2 = "";
    // Continue as if the NULL argument was the empty string.
  }
  ...

Minor: chars_compared not needed.

Simply enough to count down num and compare it to 0.

int strncmpci(const char * str1, const char * str2, size_t num) {
  ....
  while (num > 0 ...) {
    num--;
    ...
  }
...

Pedantic: int overflow

tolower((int)(*str1)) - tolower((int)(*str2)) risks int overflow (UB) on those long forgotten machine where UCHAR_MAX > INT_MAX. IMO, not a real concern - just here for the record.


Much of this in pitfalls to watch out for when doing case insensitive compares including a fast version should one care to code their own tolower tables.

\$\endgroup\$
6
  • \$\begingroup\$ Regarding casting to int, it is true that the standard says "In all cases the argument is an int, the value of which shall be representable as an unsigned char or shall equal the value of the macro EOF" and often ctype.h functions cast to unsigned char internally for this reason, but there's no guarantee. Arguably, this is not strncmp's job to fix though. Since the parameters to ctype.h functions is int, I see nothing wrong with the cast. It's always good practice to be explicit with all type conversions. \$\endgroup\$
    – Lundin
    Jan 29, 2021 at 9:25
  • 1
    \$\begingroup\$ @Lundin The issue about the (int) cast is not if it is wrong or not . When the value passed is less than 0 is the issue. is...() expects value in the unsigned char range (or EOF). The characters of the string, to match str..() functions should get read as if "each character shall be interpreted as if it had the type unsigned char" (C17 7.24.1 3). By using is...(*(unsigned char * )str1), both of these concerns are well handled unlike is...((int)*str1). Should code still want to use the (int) to fulfil your "good practice", it should be is...((int) *(unsigned char * )str1). \$\endgroup\$ Jan 29, 2021 at 13:10
  • \$\begingroup\$ @Lundin "often ctype.h functions cast to unsigned char internally" is a problem as with a common EOF == -1, a cast to unsigned char implies the result of is...(EOF) is the same as is...(255) More likely is the value is anded with 511 and the [0-511] result is then looked up in a table that is nearly the same in both its halves. IAC, that is an implementation issue. \$\endgroup\$ Jan 29, 2021 at 13:16
  • 1
    \$\begingroup\$ @Lundin When EOF == -1 (very common), c >= -128 && c < 256 ? __ctype_tolower[c] : c , EOF is not returned, but __ctype_tolower[-1] - (hmm depends on type of __ctype_tolower[]). When the character of the string is 255 and read with char * (and char is signed), the wrong table entry is used - hence the value in accessing via unsigned char *. \$\endgroup\$ Jan 29, 2021 at 13:40
  • 2
    \$\begingroup\$ Hmm nevermind, they actually do use EOF == -1 and use that a a valid table look-up. The code isn't exactly easy to read :) \$\endgroup\$
    – Lundin
    Jan 29, 2021 at 13:40
4
\$\begingroup\$
  • Overall, checking byte by byte is fast on 8/16 bit CPUs since they typically don't care about alignment, but slow on 32 bit or larger CPUs. So the byte by byte loop is fine if low-end MCUs are the target, but naive if 32 bitters are used.

  • Since you are aiming for a minimalistic implementation, then checking parameters for null is unnecessary bloat. Such error handling should be left to the caller. On smaller systems this leads to two unnecessary 16 bit index registers comparisons. On high-end systems this leads to unnecessary branches.

    When do you expect strings to point at NULL on a low-end microcontroller system anyway? It's a rare use-case scenario and you'd probably only end up there if you have bugs elsewhere in the code. If you are concerned about that, then toss in some assert in the calling code of the debug build.

  • int return type is compatible with standard C, but unnecessarily slow on 8 bit MCUs. int8_t would give a performance gain. The ABI/calling convention of various different 8-bit compilers tend to be wildly different from each other though. It might make sense to make specific MCU ports if library quality code is desired.

  • You can drop chars_compared and instead down-count num. Since the loop in this case is not trivial, even a modern compiler might struggle to optimize that part. On 8 bitters, "branch if zero" instructions are somewhat faster than "branch if equal/less" etc.

  • tolower/toupper are inefficient and come with the "don't pass negative value" hiccup mentioned in another review. Switching the internal types used from const char* to const unsigned char* shaved down 7 instructions in total with AVR gcc compiler. Use char and it does some strange temporary variable passing, that goes away if you use unsigned char.

    Without any deeper analysis, I suspect this is mostly related to the dysfunctional int API of the ctype.h functions, which is not well-designed for 8 bit microcontrollers.

    If you need maximum portability, ok then you need to call tolower. If you want speed, however... this is your code, disassembled on a crappy AVR:

    mov __tmp_reg__,r24
    lsl r0
    sbc r25,r25
    call tolower
    movw r16,r24
    mov r24,r15
    lsl r15
    sbc r25,r25
    call tolower
    movw r18,r16
    sub r18,r24
    sbc r19,r25
    movw r24,r18
    

    Eww. Without even expanding those functions to see what's inside, function calls in themselves are quite expensive on 8 bitters. Now consider writing a completely non-portable version:

    static char tolower_fast (const char* str)
    {
      return (*str>='A' && *str<='Z') ? (*str + 'a'-'A') : *str;
    }
    ...
    ret_code = tolower_fast(str1) - tolower_fast(str2);
    

    AVR gcc gives me:

    .L5:
          mov __tmp_reg__,r18
          lsl r0
          sbc r19,r19
    
          ldi r24,lo8(-65)
          add r24,r25
          cpi r24,lo8(26)
          brsh .L6
          subi r25,lo8(-(32))
    .L6:
          sub r18,r25
          sbc r19,__zero_reg__
          sbrc r25,7
          inc r19
    

    where the 5 instructions from ldi in the middle is roughly the tolower_fast function inlined. Yeah it's completely non-portable and won't support EBCDIC at all. But it's running in circles around the original slow code.

    And then if we also drop char:

    int strncmpci1(const char * s1, const char * s2, size_t num)
    {
      const uint8_t* str1 = s1;
      const uint8_t* str2 = s2;
    ...
      ret_code = tolower_faster(str1) - tolower_faster(str2);
    

    with the same function just unsigned types:

    static uint8_t tolower_faster (const uint8_t* str)
    {
      return (*str>='A' && *str<='Z') ? (*str + 'a'-'A') : *str;
    }
    

    Then it boils down to this:

    .L5:
          ldi r19,0
          ldi r24,lo8(-65)
          add r24,r25
          cpi r24,lo8(26)
          brsh .L6
          subi r25,lo8(-(32))
    .L6:
          sub r18,r25
          sbc r19,__zero_reg__
    

    I'd even dare call it library-quality code now. :)

\$\endgroup\$
3
  • \$\begingroup\$ Why is tolower_fast completely non-portable? Is it non-portable for machines that encode strings in a non-ascii way? \$\endgroup\$ Jan 30, 2021 at 14:11
  • \$\begingroup\$ "int return type is compatible with standard C, but unnecessarily slow on 8 bit MCUs. int8_t would give a performance gain." --> hmm, perhaps return int_fast8_t rather than "make specific MCU ports". IAC.Using an 8-bit return for strncmpci1() (if you meant that)increases OF/conversion concerns with those pesky characters outside the 0-127 range. \$\endgroup\$ Jan 31, 2021 at 2:15
  • \$\begingroup\$ @chux-ReinstateMonica It would specifically be slightly slower on AVR, at least on most ABIs. int_fast8_t is indeed a good option. \$\endgroup\$
    – Lundin
    Feb 1, 2021 at 7:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.