On Wed, Nov 4, 2020 at 2:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Magnus Hagander <magnus@hagander.net> writes:
> > [ remove_option_o_2.patch ]
>
> This seems committable to me now, although ...
>
> > On Mon, Nov 2, 2020 at 6:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Magnus Hagander <magnus@hagander.net> writes:
> >>> Initially I kept the dynamic argv/argc in even though it's now
> >>> hardcoded, in case we wanted to add something back. But given the way
> >>> it looks now, perhaps we should just get rid of BackendRun()
> >>> completely and directly call PostgresMain()? Or keep BackendRun() with
> >>> just setting the TopMemoryContext, but removing the dynamic parts?
>
> >> I'd be inclined to keep it as-is for now. It's not adding any significant
> >> amount of cycles compared to the process fork, so we might as well
> >> preserve flexibility.
>
> ... looking at this again, BackendRun certainly looks ridiculously
> over-engineered for what it still does. If we keep it like this, we
> should at least add a comment along the lines of "We once had the
> ability to pass additional arguments to PostgresMain, and may someday
> want to do that again". But I wouldn't object to getting rid of the
> dynamic construction of the arg array, and the debug output too.
Yeah, looking at it again, I agree. PFA an updated patch, which I'll
go ahead and push shortly.
I do noticed when looking through this -- the comment before the function says:
* returns:
* Shouldn't return at all.
* If PostgresMain() fails, return status.
I'm pretty sure that's incorrect in the current branches as well,
since it's a void function it will never return anything. Pretty sure
it should just have the first point and not the second one there, or
is this trying to convey some meaning I'm just not getting?
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/