Re: [PATCH v2 6/8] refs: add update_refs for multiple simultaneous updates
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Sep 3, 2013, 04:43 UTC
- Message-ID
- <522568D9.3080808@alum.mit.edu>
- In-Reply-To
- <5224C8C6.4050500@kitware.com>
On 09/02/2013 07:20 PM, Brad King wrote:
Show 11 quoted lines
> On 09/01/2013 02:08 AM, Junio C Hamano wrote: >>> Though the refs themeselves cannot be modified together in a single >> >> "themselves". > > Fixed. > >> I notice that we are using an array of structures and letting qsort >> swap 50~64 bytes of data > > Michael suggested this too, so fixed.
Hmmm, I see that you changed the signature of update_refs() to take an array of pointers. My suggestion was unclear, but I didn't mean that the function signature had to be changed. Rather, I meant that *within* the function, you could have created an array of pointers to the structures in the input array and thereafter accessed it via the pointers:
int update_refs(const char *action, const struct ref_update *updates_orig,
int n, enum action_on_err onerr)
{
[...]
struct ref_update **updates;[...] updates = xcalloc(n, sizeof(*updates)); for (i = 0; i < n; i++) updates[i] = &updates_orig[i]; [...] }
However, your approach is also fine. It will typically involve more malloc()s but smaller memcpy()s (i.e., via ALLOC_GROW()) at the caller, and since usually the number of ref_updates being done at one time will be limited anyway, I don't see a reason to prefer one version over the other.
Thanks for making the change.
Michael
-- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/