On 5 January 2013 16:58, Magnus Hagander <mag...@hagander.net> 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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers