If a line is longer than INI_MAX_LINE, consume input up to next newline #188
Conversation
…input up to the next newline.
ini.c
Outdated
| if (offset == max_line - 1 && line[offset - 1] != '\n') { | ||
| /* Read 1 byte to check the stream isn't at EOF and that the next | ||
| character isn't a newline -- both of which would be okay. */ | ||
| if (reader(abyss, 2, stream) != NULL && abyss[0] != '\n') { |
There was a problem hiding this comment.
I wonder -- would it be better to avoid the initial reader call here and just have a single while loop below? I think it'd be okay to still flag an error in the "newline came immediately after" (the line didn't fit, after all). Perhaps something like this (untested):
if (offset == max_line - 1 && line[offset - 1] != '\n') {
/* Consume stream up to the next newline. */
while (reader(abyss, sizeof(abyss), stream) != NULL) {
if (!error)
error = lineno;
if (abyss[strlen(abyss) - 1] == '\n')
break;
}
}
There was a problem hiding this comment.
Yes, why not - your way is more concise. I still kind of prefer my way, but I think this falls into the category of personal preference :) The important thing is that it doesn't flag a false positive when the last line of the INI file isn't terminated with a newline (which your way addresses).
I have never done a pull request on github so please bear with me. How would we normally proceed? Do I edit my branch, or would you edit the pull request? Let me know what you need from me.
There was a problem hiding this comment.
This pull request is tracking your branch, so if you edit (push to) your branch on your fork, this PR will update. Just like you've already done with the recent commits.
If you could please make two more changes, I'll go ahead and merge this:
- b0ffcbb: don't access line if reader returns NULL (it may not be valid to do so)
- ab6b614: simplify the discard loop, per my previous comment
These still pass all the tests.
Thanks again!
There was a problem hiding this comment.
I've merged your commits into my branch so, if you're happy with it, please proceed!
…op and therefore not flagging long line errors.
…es over long lines. Co-authored-by: Ben Hoyt <[email protected]>
2 participants
Opening this PR for code review. Thanks @msquire.