Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Wed, Sep 06, 2017 at 08:12:24PM +0200, Martin Ågren wrote: > On 5 September 2017 at 23:26, Junio C Hamano wrote: > > Jeff King writes: > > > >> I noticed the HEAD funniness, too, when looking at this earlier. I agree > >> with Junio that it's not quite

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

Martin Ågren writes: > Junio: The topic is in pu as ma/split-symref-update-fix. Re-roll or new > topic or as a separate "patch 4/3" for you to place on top, any > preference? Until a topic hits 'next', you solely own it and it is mostly up to you how to go about

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On 5 September 2017 at 23:26, Junio C Hamano wrote: > Jeff King writes: > >> I noticed the HEAD funniness, too, when looking at this earlier. I agree >> with Junio that it's not quite consistent with the general rule of >> "string list items point to their

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

Jeff King writes: > I noticed the HEAD funniness, too, when looking at this earlier. I agree > with Junio that it's not quite consistent with the general rule of > "string list items point to their refnames", but I don't think it > matters in practice. Yup, we are on the same

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Sep 05, 2017 at 07:24:18PM +0200, Martin Ågren wrote: > > And from that point of view, doesn't split_head_update() wants a > > similar fix? It attempts to insert "HEAD", makes sure it hasn't > > been inserted and then hangs a new update transaction as its util. > > It is not wrong per-se

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On 5 September 2017 at 12:02, Junio C Hamano wrote: > Martin Ågren writes: > >> On 30 August 2017 at 04:52, Michael Haggerty wrote: >>> v3 looks good to me. Thanks! >>> >>> Reviewed-by: Michael Haggerty >>

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

Martin Ågren writes: > On 30 August 2017 at 04:52, Michael Haggerty wrote: >> v3 looks good to me. Thanks! >> >> Reviewed-by: Michael Haggerty > > Thank _you_ for very helpful feedback on the earlier versions. > > Martin Yes,

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Sep 05, 2017 at 11:03:36AM +0200, Michael Haggerty wrote: > > It feels pretty dirty, though. It would certainly be a bug if we ever > > decided to switch affected_refnames to duplicate its strings. > > > > So given that your solution is only a constant-time factor worse in > > efficiency,

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Sep 5, 2017 at 10:45 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 07:18:22PM +0200, Martin Ågren wrote: > >> [...] >> Rearrange the handling of `referent`, so that we don't add it directly >> to `affected_refnames`. Instead, first just check whether `referent` >> exists

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Aug 29, 2017 at 07:18:22PM +0200, Martin Ågren wrote: > Observe that split_symref_update() creates a `new_update`-object through > ref_transaction_add_update(), after which `new_update->refname` is a > copy of `referent`. The difference is, this copy will be freed, and it > will be freed

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On 30 August 2017 at 04:52, Michael Haggerty wrote: > v3 looks good to me. Thanks! > > Reviewed-by: Michael Haggerty Thank _you_ for very helpful feedback on the earlier versions. Martin

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

v3 looks good to me. Thanks! Reviewed-by: Michael Haggerty Michael

[PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

split_symref_update() receives a string-pointer `referent` and adds it to the list of `affected_refnames`. The list simply holds on to the pointers it is given, it does not copy the strings and it does not ever free them. The `referent` string in split_symref_update() belongs to a string buffer in