Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44085 )
Change subject: lib/string: Add standard strstr() function ......................................................................
Patch Set 1:
(2 comments)
Please clarify what modifications you are suggesting.
https://review.coreboot.org/c/coreboot/+/44085/1/src/lib/string.c File src/lib/string.c:
https://review.coreboot.org/c/coreboot/+/44085/1/src/lib/string.c@169 PS1, Line 169: for (; *haystack; haystack++) {
Ack. […]
I am not sure if the suggestion is to make a one-time comparison, and bail out before the for loop in case needle is longer, or whether the suggestion is to replace the exit condition of the for loop with the length comparison.
The latter is only an improvement if haystack is shorter than about double the size of the needle, which it will rarely be, and the former only improves the even more unlikely case of the given haystack being shorter than the needle. I think as Julius, that it is not worth optimizing for such degenerate cases. In particular when the cost of the latter interpretation being repeated strlen over the potentially long haystack, making the runtime O(h^2) instead of O(h*n) for the rather common case of the haystack being significantly longer than the needle.
https://review.coreboot.org/c/coreboot/+/44085/1/src/lib/string.c@170 PS1, Line 170: if (!strncmp(haystack, needle, needle_len))
I still think you should just use strcmp() here so you can avoid the strlen(needle) as well, btw (al […]
In the other cl you ask "Why not just strcmp()?". I have to say, I do not see how this can be implemented using strcmp(). We are trying to find our if a null terminated string, needle, is equal to a number of characters in the middle of a longer string. That is, there will not be any null termination after the "window" of the haystack string that we are interested in, so naively calling strcmp() will never return a match, unless the needle happens to be found at the very end of the haystack.
Are you proposing mutating the haystack, to add zeros "in the middle" of the string, and put back the original character afterwards, that we cannot do as the input is const.
Am I missing something obvious?