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 istate = { NULL };
> > >  
> > >   init_revisions(&rev, NULL);
> > > + copy_pathspec(&rev.prune_data, ps);
> > 
> > This moved here... because...
> > 
> > >  
> > >   set_alternate_index_output(stash_index_path.buf);
> > >   if (reset_tree(&info->i_tree, 0, 0)) {
> > 
> > ... this `if` block could jump to...
> > 
> > 
> > > @@ -1050,7 +1058,6 @@ static int stash_working_tree(struct stash_info 
> > > *info, struct pathspec ps)
> > >   }
> > >   set_alternate_index_output(NULL);
> > >  
> > > - rev.prune_data = ps;
> > >   rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
> > >   rev.diffopt.format_callback = add_diff_to_buf;
> > >   rev.diffopt.format_callback_data = &diff_output;
> > > @@ -1089,12 +1096,13 @@ static int stash_working_tree(struct stash_info 
> > > *info, struct pathspec ps)
> > 
> > ... this point (the `done:` label is *just* one line further up, and this
> > is a static diff, so we cannot just increase the context when we need to
> > see more, unlike, say, GitHub PRs) and...
> > 
> > >   discard_index(&istate);
> > >   UNLEAK(rev);
> > >   object_array_clear(&rev.pending);
> > > + clear_pathspec(&rev.prune_data);
> > 
> > ... we add this call here.
> > 
> > However, we would not have needed to move the initialization of
> > `rev.prune_data`, I don't think, because `init_revision()` zeros the
> > entire struct, including `prune_data`, which would have made
> > `clear_pathspec()` safe to call, too.
> 
> 'clear_pathspec()' doesn't actually check whether the parameter passed
> to it is NULL or not before dereferencing it.

In this case, it does not need to check for NULL, as `&rev.prune_data`
will always be non-NULL: `rev`'s `prune_data` field is of type `struct
patchspec`, i.e. *not* a pointer (in which case the type would be `struct
pathspec *`). See for yourself:

        https://github.com/git/git/blob/v2.21.0/revision.h#L91

Ciao,
Dscho

Reply via email to