Eric Sunshine writes:
> On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva
> <rafaeloliveira.cs@xxxxxxxxx> wrote:
>> Add worktree_prune_reason() to allow a caller to discover whether a
>> worktree is prunable and the reason that it is, much like
>> worktree_lock_reason() indicates whether a worktree is locked and the
>> reason for the lock. As with worktree_lock_reason(), retrieve the
>> prunable reason lazily and cache it in the `worktree` structure.
>>
>> Signed-off-by: Rafael Silva <rafaeloliveira.cs@xxxxxxxxx>
>> ---
>> diff --git a/worktree.c b/worktree.c
>> @@ -245,6 +246,25 @@ const char *worktree_lock_reason(struct worktree *wt)
>> +const char *worktree_prune_reason(struct worktree *wt, timestamp_t expire)
>> +{
>> + struct strbuf reason = STRBUF_INIT;
>> + char *path;
>
> The `path` variable is uninitialized...
>
>> + if (should_prune_worktree(wt->id, &reason, &path, expire))
>> + wt->prune_reason = strbuf_detach(&reason, NULL);
>
> ...but the very first thing should_prune_worktree() does is
> unconditionally set it to NULL, so it's either NULL or an allocated
> string regardless of the value returned by should_prune_worktree()...
>
>> + strbuf_release(&reason);
>> + free(path);
>
> ...which makes the behavior of `free(path)` deterministic. Good.
>
> I'm on the fence about whether or not we should initialize `path` to NULL:
>
> char *path = NULL;
>
> just to make it easier for the next person to reason about it without
> having to dig into the implementation of should_prune_worktree(). This
> is a really minor point, though, not worth a re-roll.
This is a good point and totally agreed. I'm going to include this
change in the next revision.
--
Thanks
Rafael
