Unable to return NULL when character not found in the function xstrchr

Question

I am not able to make use of the else condition in main(). The code doesn't work when character is not found in string. How should I modify the return statement or the else condition in main ?

#include <stdio.h>

char *xstrchr(char *string, char ch);

void main() {
    char str[20], ch;
    printf("Enter a string :");
    gets(str);
    printf("Enter character to search in string :");
    scanf("%c", &ch);

    char *r = xstrchr(str, ch);
    printf("%c is stored at %d\n", *r, r);
    if (r != NULL)
        printf("Character '%c' found at %d index.\n", ch, (r - str + 1));
    else
        printf ("Character not found.");
}

char *xstrchr(char *s, char ch) {
    int flag = 0, i = 0;
    while (*(s + i) != '\0') {
        if (*(s + i) == ch) {
            flag = 1;
            return (s + i);
        }
        i++;
    }
    if (flag == 0) {
        return NULL;
    }
}

`*(s + i)` should be written as `s[i]`. The `flag` variable is useless - it is always 0, except when set just before the `return`. `return` is not a function and doesn't require parentheses. `%d` cannot be used to print an address, use `%p`, use `size_t` as an index to strings, `main` must return `int`. — Antti Haapala -- Слава Україні, Mar 14 '19 at 05:50
Never ***ever*** use `gets`! It's a [dangerous function](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used). Use e.g. [`fgets`](https://en.cppreference.com/w/c/io/fgets) instead, but be aware of the differences between the functions. — Some programmer dude, Mar 14 '19 at 05:50
"The code doesn't work" is no useful description of a problem. Please always tell us what you expect, what you get and what input you used for test. — Gerhardh, Mar 14 '19 at 06:53

chqrlie · Accepted Answer · 2019-03-14T06:14:55.700

Your function does return NULL when the character is not found, but you have undefined behavior in main() when it does so as you dereference r in the printf statement before testing for a NULL result, causing your program to crash.

There are other problems:

the prototype for main should be int main(void) or int main(int argc, char *argv[]) or possibly int main().
you must nos use gets(). This function has been removed from the C language because it cannot be used safely. If the user types more than 19 characters at the prompt, your program will have undefined behavior as gets() will write beyond the end of the array.
pointers should not be printed with %d, you should write:
```
printf("%c is stored at %p\n", *r, (void *)r);
```
the difference of 2 pointers has type ptrdiff_t, which mau be larger than int and should not be printed with %d. The standard format for this is %td, but many C libraries do not support this, so converting the difference to int may be preferable:
```
printf("Character '%c' found at %d index.\n", ch, (int)(r - str + 1));
```
index values start at 0 in C. It is confusing to refer to the first element of an array as index 1.
you should test the return values of fgets() and scanf() to detect premature end of file or other input failures.
index values may exceed the range of type int. You should use type size_t for i in xstrchr().
flag is not used in the xstrchr function: the update code and the test are redundant and can be removed.
*(s + i) can be written s[i], which is much more readable. Unless you are required to use the pointer arithmetic syntax explicitly, you should use the bracketed syntax.

Here is a corrected version:

#include <stdio.h>

char *xstrchr(char *string, char ch);

int main() {
    char str[200], ch;
    printf("Enter a string: ");
    if (!fgets(str, sizeof str, stdin))
        return 1;
    printf("Enter character to search in string: ");
    if (scanf("%c", &ch) != 1)
        return 1;
    char *r = xstrchr(str, ch);
    if (r != NULL) {
        printf("Character '%c' found at %d index.\n", ch, (int)(r - str));
    else
        printf("Character not found.\n");
    return 0;
}

char *xstrchr(char *s, char ch) {
    size_t i = 0;
    while (s[i] != '\0') {
        if (s[i] == ch) {
            return s + i;
        }
        i++;
    }
    return NULL;
}

Finally, the xstrchr() function does not completely match the specification of the standard strchr() function:

the argument type for ch is int, but its value is converted to char for comparison purposes.
the argument type for s is const char * as the string is not modified by this function, but the return type is char * so the return value must then be cast as (char *), which is an unfortunate but necessary step.
the null terminator at the end of the string will be matched if ch has a null value.
incrementing s instead of using an index variable is an alternative many C programmers would prefer.

Here is a modified version:

char *xstrchr(const char *s, int ch) {
    for (;; s++) {
        if (*s == (char)ch)
            return (char *)s;
        if (*s == '\0')
            return NULL;
    }
}

Unable to return NULL when character not found in the function xstrchr

1 Answers1