René Scharfe <l.s.r@xxxxxx> writes:
>> - /*
>> - * NEEDSWORK: It's possible for strchrnul() to scan beyond the range
>> - * given by len. However, current callers are safe because they compute
>> - * len by scanning a NUL-terminated block of memory starting at msg.
>> - * Nonetheless, it would be better to ensure the function does not look
>> - * at msg beyond the len provided by the caller.
>> - */
>> while (line && line < msg + len) {
>> - const char *eol = strchrnul(line, '\n');
>> + char *eol = (char *) line;
>> + for (size_t i = 0; i < len && *eol && *eol != '\n'; i++) {
>> + eol++;
>> + }
>
> This uses the pointer eol only for reading, so you can keep it const.
>
> The loop starts counting from 0 to len for each line, which cannot be
> right. find_header_mem("headers\nfoo bar", 9, "foo", &len) would still
> return "bar" instead of NULL.
>
> You could initialize i to the offset of line within msg instead (i.e.
> i = line - msg). Or check eol < msg + len instead of i < len -- then
> you don't even need to introduce that separate counter.
>
> Style nit: We tend to omit curly braces if they contain only a single
> statement.
All true. As we already use an extra variable 'i' for counting, we
can do without eol and reference line[i] instead, which would make
the whole thing something like
while (line && line < msg + len) {
size_t i;
for (i = 0;
i < len && line[i] && line[i] != '\n';
i++)
;
if (key_len < i &&
!strncmp(line, key, ken_len) &&
linhe[key_len] == ' ') {
*out_len = i - key_len - 1;
return line + key_len + 1;
}
line = line[i] ? line + i + 1 : NULL;
}
which is not too bad, simply because the original already needed to
know the length of the current line and due to lack of this "i" you
introduced, it used "eol-line" instead. Now you have "i", the code
may get even simpler by getting rid of "eol".
