Re: [PATCH v3 4/4] wt-status.c: Set the committable flag in the collect phase.

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 24, 2018, 21:02 UTC
Message-ID: <xmqqzhw6r4m1.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <2295579.U4Xb9QnJqG@thunderbird>

Stephen Smith <ischis2@cox.net> writes:

Show 12 quoted lines

> I can see three solutions and could support any of the three:
> 1) Move the free calls to run_status() and cmd_status().
> 2) Move the calls calls to wt_status_print since that is the last function 
> from wt_status.c that is called befor the structure goes out of scope in  
> run_status() and cmd_status().
> 3) Add a new wt_collect*() function to free the variables. This would have an 
> advantage that the free calls could be grouped in on place and not done in to 
> functions.  A second advantage is that the free calls would be located where 
> the pointers are initialized.  
>
> Personally I like solutions 1 and 3 over 2.
> What do others think?

I think freeing at the top level caller (i.e. #1) once it finished using the information collected would make the most sense---it initiated the collection, then fed the collected info to shower, and now it knows it is done with the pieces of memory it used to make these two parts communicate with each other.

And for keeping multiple "pieces of memory" as a unit, introducing a helper is a good technique (i.e. #3); but I view that mostly as an implementation detail of #1.

Thanks.