Can this function be any safer ? Looking for tips and your thoughts !!!

Question

Hi everyone, this is somewhat of an odd question.

I wrote a C function. Its 'like' strchr / strrchr. It's supposed to look for a character in a c-string, but going backwards, and return a pointer to it. As c strings are not "null initiated", it also takes a third parameter 'count', indicating the number of chars it should look backwards.

/*
*s: Position from where to start looking for the desired character.
*c: Character to look for.
*count: Amount of tests to be done
*
* Returns NULL if c is not in (s-count,s)
* Returns a pointer to the occurrence of c in s.
*/
char* b_strchr(const char* s,int c,size_t count){

    while (count-->0){

        if (*s==c) return s;
        s--;
     }
     return NULL;
}

I have done some testing on it, but Do you see any flaws in it? Security issues or so? Any enhancements? Could it be improved? And more important: Is this a bad idea?

Some usage.

    char* string = "1234567890";

    printf("c: %c\n",*b_strchr(string+9,'5',10));//prints 5

    printf("c: %c\n",*b_strchr(string+6,'1',7));//prints 1

EDIT: New interface, some changes.

/*
* from:  Pointer to character where to start going back.
* begin: Pointer to characther where search will end.
*
* Returns NULL if c is not between [begin,from]
* Otherwise, returns pointer to c.
*/
char* b_strchr(const char* begin,int c,const char* from){


    while (begin<=from){

        if (*from==c) return from;
        from--;
     }
     return NULL;
}

Answer 1

The code has an esoteric interface - pass in a pointer to the last character of the string and the length of the string. That will lead to problems using it.

(Alternatively, the code has a bug - you should add count to s before the loop.)

Answer 2

It's better with the edit, but the interface is still surprising. I'd put the begin parameter (the haystack being searched) as the first parameter, the c parameter (the needle being searched for) second, and the from parameter (start position of the search) third. That order seems to be idiomatic across a fairly large set of APIs.

Answer 3

If begin is from, the current code will always return begin, which is not what you want. The code after the loop can just be return NULL. And instead of begin != from in the loop condition, I would use begin < from otherwise you will pointer arithmetic overflow when someone mixes up the order of the parameters.

Edit: on second thought since you want [begin, from] inclusive it should be begin <= from