Re: [PATCH v2] stash: pass pathspec as pointer

2019-03-15 Thread Thomas Gummerer
On 03/13, Johannes Schindelin wrote: > Hi Thomas, > > On Tue, 12 Mar 2019, Thomas Gummerer wrote: > > > On 03/12, Johannes Schindelin wrote: > > > However, we would not have needed to move the initialization of > > > `rev.prune_data`, I don't think, because `init_revision()` zeros the > > > entir

Re: [PATCH v2] stash: pass pathspec as pointer

2019-03-13 Thread Johannes Schindelin
Hi Thomas, On Tue, 12 Mar 2019, Thomas Gummerer wrote: > On 03/12, Johannes Schindelin wrote: > > > On Mon, 11 Mar 2019, Thomas Gummerer wrote: > > > [...] > > > @@ -1042,6 +1049,7 @@ static int stash_working_tree(struct stash_info > > > *info, struct pathspec ps) > > > struct index_state ist

Re: [PATCH v2] stash: pass pathspec as pointer

2019-03-12 Thread Junio C Hamano
Thomas Gummerer writes: >> I see that you added the `const` keyword. While it does not hurt, I would >> probably not have bothered... > > That's fair, I went with what seemed most common in the codebase. > More than half the parameters seem to be using "const struct > pathspec", so that seems to

Re: [PATCH v2] stash: pass pathspec as pointer

2019-03-12 Thread Thomas Gummerer
On 03/12, Johannes Schindelin wrote: > Hi Thomas, > > On Mon, 11 Mar 2019, Thomas Gummerer wrote: > > > Passing the pathspec by value is potentially confusing, as the copy is > > only a shallow copy, so save the overhead of the copy, and pass the > > pathspec struct as a pointer. > > Not only co

Re: [PATCH v2] stash: pass pathspec as pointer

2019-03-12 Thread Johannes Schindelin
Hi Thomas, On Mon, 11 Mar 2019, Thomas Gummerer wrote: > Passing the pathspec by value is potentially confusing, as the copy is > only a shallow copy, so save the overhead of the copy, and pass the > pathspec struct as a pointer. Not only confusing, but also wasteful ;-) > In addition use copy_

Re: [PATCH v2] stash: pass pathspec as pointer

2019-03-11 Thread Junio C Hamano
Thomas Gummerer writes: >> Good catch on both acconts. I'll send a new patch soon, adding the >> clear_pathspec() calls and rebasing it on top of Dscho's fix. > > Here it is. Thanks for the review of the first round Junio! > > This is on top of Dscho's series at > applied to ps/stash-in-c. Th

[PATCH v2] stash: pass pathspec as pointer

2019-03-11 Thread Thomas Gummerer
Passing the pathspec by value is potentially confusing, as the copy is only a shallow copy, so save the overhead of the copy, and pass the pathspec struct as a pointer. In addition use copy_pathspec to copy the pathspec into rev.prune_data, so the copy is a proper deep copy, and owned by the revis