On Fri, Jun 29, 2012 at 4:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Turned out to be a bit more work than I thought, since the current
>> parser reads pg_hba byte by byte, and not line by line. So I had to
>> change that. See attached, seems reasonable?
>
> A couple of comments:
>
> * In some places you have "if ((c = *(*lineptr)++) != '\0')" and in other
> places just "if ((c = *(*lineptr)++))". This should be consistent (and
> personally I prefer the first way).
>
> * I'm not sure that this conversion is right:
>
> ! if (c != EOF)
> ! ungetc(c, fp);
> ---
> ! if (c != '\0')
> ! (*lineptr)--;
>
> In the file case, it's impossible to push back EOF, and unnecessary
> since another getc will produce EOF again anyway. In the string case,
> though, I think you might need to decrement the lineptr unconditionally,
> else next call will run off the end of the string no?
>
> * This bit seems a bit ugly, and not Windows-aware either:
>
> ! /* We don't store the trailing newline */
> ! if (rawline[strlen(rawline)-1] == '\n')
> ! rawline[strlen(rawline)-1] = '\0';
> !
>
> It might be better to strip trailing \n and \r from the line immediately
> upon read, rather than here.
I re-found this branch that I had completely forgotten about, when
cleaning up my reposiroty. oops.
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...
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/