On 5 January 2013 16:58, Magnus Hagander <[email protected]> wrote:
> Attached is an updated version of the patch, per the comments from Tom
> and rebased on top of the current master. Since it's been a long time
> ago, and some code churn in the area, another round of review is
> probably a good thing...
>
I took a look at this patch, and it seems to be in pretty good shape.
It applies cleanly to head, and seems to work as advertised/discussed.
I have a couple of comments on the code...
In next_token(), in the case of an overlong token, this change looks wrong:
/* Discard remainder of line */
! while ((c = getc(fp)) != EOF && c != '\n')
! ;
break;
}
--- 188,195 ----
errmsg("authentication file token too long,
skipping: \"%s\"",
start_buf)));
/* Discard remainder of line */
! while ((c = (*(*lineptr)++)) != '\0' && c != '\n')
! (*lineptr)++;
break;
It appears to be incrementing lineptr twice per loop iteration, so it
risks missing the EOL/EOF and running off the end of the buffer.
Nitpicking, at the end of the loop you have:
! c = (**lineptr);
! (*lineptr)++;
perhaps for consistency with the preceding code, that should be "c =
(*(*lineptr)++)". Personally, I'd also get rid of the outer sets of
brackets in each of these expressions and just write "c =
*(*lineptr)++", since I don't think they add anything.
Finally, the comment block for tokenize_file() needs updating, now
that it returns three lists.
Regards,
Dean
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
