Re: [PATCH v7 1/3] worktree: add top-level worktree.c

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 22, 2015, 01:17 UTC
Message-ID: <xmqq7fnj6yn7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAPig+cQjhoVhQtP4_ZYTyLX3-GBXRx2eKM8Yv1=i=pSg=ZuM=Q@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 7 quoted lines

> Making the last entry a NULL means get_worktrees() would have to
> return an array of pointers rather than an array of structures, which
> is more syntactically noisy, and complex since it's harder to reason
> about pointer-to-pointer. In my mind, at least, the simplicity of the
> array of structures approach (even with the slight ugliness of the
> dummy sentinel) outweighs the complexity of the array-of-pointers
> approach.

Hmph, I think the bog-standard way in our code is an array of pointers. An array of structures have a few downsides:

 - You have to copy a lot when you do realloc(3);

 - You have to move a lot when you insert a new element;

 - Individual structure cannot be pointed at by a pointer sensibly.
   Passing &worktree[4] to a function that expects "struct worktree *"
   is unsafe unless you are sure nobody is mucking with the worktree[]
   array.

For the read-only operation "worktree list", the last one may not matter much because you would build all the elements before doing anything else, but once you want to run this inside a library and maintain the in-core forest of worktrees that are in sync with what your running process does (i.e. create a new or destroy an existing worktree), it may become problematic.