string manipulation, which version of those 2 functions is better?

Question

I wrote my own version of endswith just like in high-level programming languages in C which I would like reviews on.

There are 2 versions. One is my own and the other is from ccan. For some reason, I feel that ccan's version is better than mine until proven wrong.

ccan/str's version:

static inline bool strends(const char *str, const char *postfix)
{
 if (strlen(str) < strlen(postfix))
 return false;
 return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
}

My version:

bool strends(const char *str, const char *postfix)
{
 register const char *end = str + strlen(str) - 1;
 register const char *epostfix = postfix + strlen(postfix) - 1;
 while (end > str && epostfix > postfix)
 if (*end-- != *epostfix--)
 return false;
 return true;
}

Update This is the newer version of mine:

bool strends(const char *str, const char *postfix)
{
 register const char *end = str + strlen(str) - strlen(postfix);
 if (strlen(str) < strlen(postfix))
 return false;
 while (*end)
 if (*end++ != *postfix++)
 return false;
 return true;
}

I wonder which one is better and why (in terms of readability, performance and so on). Please be specific and provide your research if possible.

Thanks in advance.

Answer 1

In your updated version, you do basically the same as ccan (you can/should switch the first statement with the if statement). The ccan way is the way I would prefer. Code reuse is most of the time a good idea. I would rather trust the common implementations of string.h in terms of safety (It would surprise me if a bug survived 30 years of usage) and speed for typical usage (It would surprise me too, if there is a significant faster way to do it without any drawbacks for such a simple function).

Everyone reading your code has to think what is happening and if it is right. Everyone reading strcmp knows whats happening and will probably trust the correctness.

Answer 2

I like the more the first, because it works at an higher level of abstraction and it makes it clear that you are interested in comparing a part of the string with the postfix.

I'd probably introduce another variable to clearly represent that you are comparing the postfix with the latest strlen(postfix) chars of the original string.

• Fallen: Hmm, total agreed :P

Answer 3

Your version will fail when strlen(postfix)> strlen(str). Once your loop reaches the start of str, it will end the loop and return true which isn't correct.

Also, unless there is some special property about the register keyword that makes this work - comparing end to epostfix doesn't make sense to me - it will always be true or always be false depending on where the two strings are in memory.

• Fallen: My bad on that one, didn't mean it.

Answer 4

Your version has its problems: why do you use a loop? The postfix is either at the end or it is not.

The ccan version is optimal. One might think that calling strlen twice on each string is wasteful, but as long as optimisation is enabled, this gets optimised away. Having said that, I'd write it as follows, but even with -O1 this ends up more or less the same (at assembler level) as the ccan code:

int endswith(const char *s, const char *t)
{
 size_t len_s = strlen(s);
 size_t len_t = strlen(t);
 if (len_t <= len_s) {
 return strcmp(s + len_s - len_t, t) == 0;
 }
 return 0;
}

• Fallen: I know that calling strlen isn't wasteful since it has a pure attribute. Your kind of wrong about the first statement The postfix is either at the end or it is not; well, the postfix should be something like aaa while str can be like bbbaaa, you may have not thought about it harder (If I understand correctly).
• William Morris: Ok, I see now - you are doing a backwards strcmp with your loop. Sorry, I didn't read your function carefully enough.