Re: [PATCH 2/3] xfs_scrub: implement deferred description string rendering

[Eric Sandeen <sandeen@sandeen.net>]

On 11/6/19 9:51 AM, Darrick J. Wong wrote:
>> but it's not clear to me, if, this is the case, who is responsible to
>> free up the memory previously associated with the dsc->where pointer
>> here, and so, it just feels like a potential memory leak landmine
>> here.
> It's the caller's responsibility.  So far all three callers passed in
> pointers local stack variables, so the variable and the @dsc disappear
> into the aether when the function returns.
> 
>> Maybe I've got confused by the comment or didn't fully understand your
>> intention here.
> Nah, the problem is that the comment is unclear.  How about:
> 
> /*
>  * Set a new location context for this deferred-rendering string.
>  * The caller is responsible for freeing the old context, if necessary.
>  */

LGTM, maybe "if any" to indicate there may not be (probably is not) an
old context.

> Question: does this API need to return the old context?  For now the
> void return is fine under the "YAGNI" principle, since we can always add
> it later if the usage pattern changes.

Can't imagine why you'd need the old one....
 
W/ the comment-flogging,

Reviewed-by: Eric Sandeen <sandeen@redhat.com>