On Thu, Mar 19, 2015 at 6:42 AM, Asif Naeem wrote: > I do agree, pg_ctl refactoring is making current patch more complicated, > eventually we can come up with robust patch that include pg_ctl but result > patch could be big and touching lot of areas, I doubt that if such > complicated and big patch going to make its way into the repository. It > seems appropriate to do incremental work, we can refactor pg_ctl changes as > next effort. PFA update patch, it removes pg_ctl related code changes. > Please do let me know if I missed something. Thanks.
After looking closely at those things, I agree with you: having some code duplicated in two places instead of five is still a win... Now, I think having this declaration of write_stderr in restricted_token.c is confusing: +#define write_stderr(fmt, ...) fprintf(stderr, fmt, __VA_ARGS__) We could have bad surprises in the future if there is some work to link pg_ctl with the stuff of restricted_token.c. Hence, could you replace that with plain calls to fprintf(stderr, ...)? That's as well what the other files of src/common are doing, so it would make the new code more consistent with the rest.
Except those two small things, well I guess that's it for this patch.
Thank you so much. PFA v5 updated patch with above 2 changes.
The stuff for write_stderr may need a TODO item, but I think that even with that we are going to finish by fixing the call to isatty that looks indeed strange...
+1
For the backpatching, the patches sent previously here (=> http://www.postgresql.org/message-id/CAEB4t-OHNE95=n5U4ySsYkWipQsWeQuTBSJkaYJ63_1VzkzkhA@mail.gmail.com) are fine IMO. They simply consist of a copy of what is done in initdb.c. Now, perhaps we had better apply the patch duplicating the logic to all branches, including HEAD, first, see what the buildfarm says, and then finish wrapping up the refactoring patch.
PFA patch for older branches, v1 patch was not applying cleaning other than 94. Thanks.
