Re: [PATCH] Handle long lines properly

On Mon, Oct 29, 2007 at 12:17:30AM +0100, Krzysztof Oledzki wrote:
>
>
> On Mon, 29 Oct 2007, Willy Tarreau wrote:
> <CUT>
> >Could you please move this just above MAX_LINE_ARGS, a few lines
> >below,
>
> Of course.
>
> >and maybe at the same time increase MAX_LINE_ARGS ?
>
> 64? 128?

maybe 64 as a starter. I wouldn't be the one who manipulates 64 args on a single line!

> >>@@ -2323,6 +2323,15 @@ int readcfgfile(const char *file)
> >>
> >> end = line + strlen(line);
> >>
> >>+ if (*(end-1) != '\n' && strlen(line) + 1 ==
> >>sizeof(thisline)) {
> >>+ /* Check if the last char is not \n and we reached
> >>the limit. Watch out
> >>+ * for the last line without the terminating '\n'!
> >>+ */
> >
> >I think that the second part of the check is needed, as it seems to
> >me that the only case where we can encounter something other than
> >'\n' in *(end-1) is precisely when the line has been truncated.
>
> Some editors (like mcedit for example) are able to create a file without a
> terminating '\n' on the last line. I know that it is wrong but... So, I
> would like to distinguish such situation from a truncated line as the
> application should not reject such files, especially with a misleading
> error message.

OK, but then I'd like to avoid redoing strlen(line) a second time. We could then have something like this :

                 end = line + strlen(line);

+ if ((end-line == sizeof(thisline)-1 && *(end-1) != '\n') {

Also, my initial fgets() in the while() is awful, it hides an assignment. Would you please assign 'line' inside the loop ? It seems like we could even declare it there, making code auditing easier.

Thanks,
Willy Received on 2007/10/29 00:27