A simple and safe implementation for strnstr (search for a substring in the first n characters of char array)

I'd like to suggest the following implementation:

// Find an instance of substr in an array of characters
// the array of characters does not have to be null terminated
// the search is limited to the first n characters in the array.
char *strnstr(char *str, const char *substr, size_t n)
{
    char *p = str, *pEnd = str+n;
    size_t substr_len = strlen(substr);

    if(0 == substr_len)
        return str; // the empty string is contained everywhere.

    pEnd -= (substr_len - 1);
    for(;p < pEnd; ++p)
    {
        if(0 == strncmp(p, substr, substr_len))
            return p;
    }
    return NULL;
}

The rationale for the first parameter is not to be const is that you may want to use the return value pointer to modify the array in that location.

for completeness, in C++, it's possible to add an overloaded variant that's const:

const char *strnstr(const char *str, const char *substr, size_t n)
{
    return strnstr((char *)str, substr, n);
}

Any comments?

As suggested, here's a test program:

#include 
#include 
#include 

int main()
{
    char s[] = "1234567890abcdefgh";
    size_t n = sizeof(s) - 1;

    const char *patterns[] = { "efgh", "0ab", "0b", NULL };
    const char *result = NULL;
    const char *pPattern = patterns[0];

    std::cout << "array of length " << n << " is: " << s << std::endl;

    for (int i = 0; pPattern; pPattern = patterns[++i])
    {
        result = strnstr(s, pPattern, n);
        std::cout << "finding " << pPattern << " n=" << n << ": "
            << (result ? result : "(null)") << std::endl;
    }

    pPattern = patterns[0];
    result = strnstr(s, pPattern, n-1);
    std::cout << "finding " << pPattern << " n=" << n-1 << ": "
        << (result ? result : "(null)") << std::endl;
    return 0;
}

Output:

array of length 18 is: 1234567890abcdefgh
finding efgh n=18: efgh
finding 0ab n=18: 0abcdefgh
finding 0b n=18: (null)
finding efgh n=17: (null)


-
Design: The str as an array and searched up to n is inconsistent with string like functions and strstr(). Rather than "the search is limited to the first n characters in the array", I'd also expect characters in str that follow a null character are not searched. IMO, a design flaw.

Following review assumes str[i] == 0 has no special meaning.

-
Weak argument name str. str is the address of an array and maybe not a string. Calling a potential non-string str conveys the wrong idea. Suggest src, etc. When looking for sub-strings, I like needle and haystack.

-
As the C version does not change str contents, recommend making that const. "The rationale for the first parameter is not to be const is that you may want to use the return value pointer to modify the array in that location." does not apply. Just cast char * the return value. Follow strstr()s tyle.

// From C library.
      char *strstr( const char *s1, const char *s2); 

// Expected signatures: (spaced for clarity)
// C
      char *strnstr(const char *src, const char *substr, size_t n);
// C++
      char *strnstr(      char *src, const char *substr, size_t n);
const char *strnstr(const char *src, const char *substr, size_t n);

-
Using a name that is close to standard names is tricky. C reserves name with certain prefixes, etc and so does *nix, etc. Maybe use CP_strnstr() and an optional #define strnstr CP_strnstr.

-
Corner case: Returning str with if(0 == substr_len) return str; does not make sense when size == 0. I'd expect NULL.

-
Underflow possible. The length of the needle may be longer or shorter than the haystack

// add check
if (n + 1 < substr_len) {
  return NULL;
}
pEnd -= (substr_len - 1);

Minor

-
In debug mode, consider testing against NULL

char *strnstr(char *str, const char *substr, size_t n) {
  assert(str || n == 0);
  assert(substr);

StackExchange Code Review Q#132519, answer score: 3