From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Chapman Flack <chap(at)anastigmatix(dot)net>, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: ArchiveEntry optional arguments refactoring |
Date: | 2019-02-01 11:33:49 |
Message-ID: | 201902011133.t5pccj6jraqe@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
pgindent didn't like your layout with two-space indents for the struct
members :-( I thought it was nice, but oh well. This means we can do
away with the newline at each callsite, and I didn't like the trailing
comma (and I have vague recollections that some old compilers might
complain about them too, though maybe we retired them already.)
> * Use NULL as a default value where it was an empty string before (this
> required few minor changes for some part of the code outside ArchiveEntry)
Ah, so this is why you changed replace_line_endings. So the comment on
that function now is wrong -- it fails to indicate that it returns a
malloc'ed "" on NULL input. But about half the callers want to have a
malloc'ed "-" on NULL input ... I think it'd make the code a little bit
simpler if we did that in replace_line_endings itself, maybe add a
"want_dash" bool argument, so this code
if (!ropt->noOwner)
sanitized_owner = replace_line_endings(te->owner);
else
sanitized_owner = pg_strdup("-");
can become
sanitized_owner = replace_line_endings(te->owner, true);
I don't quite understand why the comments about line sanitization were
added in the callsites rather than in replace_line_endings itself. I
would rename the function to sanitize_line() and put those comments
there (removing them from the callsites), then the new argument I
suggest would not be completely out of place.
What do you think?
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment | Content-Type | Size |
---|---|---|
0001-ArchiveOpts-structure.patch | text/x-diff | 58.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2019-02-01 11:43:23 | Re: ArchiveEntry optional arguments refactoring |
Previous Message | rajan | 2019-02-01 11:24:57 | Re: where clause not working through psql command line client |