Re: ArchiveEntry optional arguments refactoring
От | Alvaro Herrera |
---|---|
Тема | Re: ArchiveEntry optional arguments refactoring |
Дата | |
Msg-id | 201902011133.t5pccj6jraqe@alvherre.pgsql обсуждение исходный текст |
Ответ на | Re: ArchiveEntry optional arguments refactoring (Dmitry Dolgov <9erthalion6@gmail.com>) |
Ответы |
Re: ArchiveEntry optional arguments refactoring
|
Список | 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
Вложения
В списке pgsql-hackers по дате отправления: