On Friday, September 7, 2018 3:31:55 PM MST Junio C Hamano wrote: > For example, I noticed that both of the old > callsites of wt_status_get_state() have free() of a few fiedls in > the structure, and I kept the code as close to the original, but I > suspect they should not be freed there in the functions in the > "print" phase, but rather the caller of the "collect" and "print" > should be made responsible for deciding when to dispose the entire > wt_status (and wt_status_state as part of it). This illustration > patch does not address that kind of details (yet). I followed the call tree back to original callers run_status() and cmd_status() in commit.c This leads to a philosophical question. We want to move the state information out of the print functions because it doesn't seem correct. For the case in question this includes the calls to free() . By doing this we seem go have traded one location that shouldn't be touching the state variables for another. 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? sps
