Hi Junio,
On Wed, 31 Jul 2019, Junio C Hamano wrote:
> "Daniel Ferreira via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > @@ -6273,12 +6257,7 @@ void diff_flush(struct diff_options *options)
> > dirstat_by_line) {
> > struct diffstat_t diffstat;
> >
> > - memset(&diffstat, 0, sizeof(struct diffstat_t));
> > - for (i = 0; i < q->nr; i++) {
> > - struct diff_filepair *p = q->queue[i];
> > - if (check_pair_status(p))
> > - diff_flush_stat(p, options, &diffstat);
> > - }
> > + compute_diffstat(options, &diffstat, q);
> > if (output_format & DIFF_FORMAT_NUMSTAT)
> > show_numstat(&diffstat, options);
> > if (output_format & DIFF_FORMAT_DIFFSTAT)
> > @@ -6611,6 +6590,20 @@ static int is_submodule_ignored(const char *path, struct diff_options *options)
> > return ignored;
> > }
> >
> > +void compute_diffstat(struct diff_options *options,
> > + struct diffstat_t *diffstat,
> > + struct diff_queue_struct *q)
> > +{
> > + int i;
> > +
> > + memset(diffstat, 0, sizeof(struct diffstat_t));
> > + for (i = 0; i < q->nr; i++) {
> > + struct diff_filepair *p = q->queue[i];
> > + if (check_pair_status(p))
> > + diff_flush_stat(p, options, diffstat);
> > + }
> > +}
>
> Hmm, (1) clearing diffstat struct to initialize, (2) looping over
> diff_queue to compute stat for each path, (3) using diffstat
> information and then (4) finally freeing the diffstat info is the
> bog-standard sequence of the user of this API. Merging step (1) and
> (2) may probably be OK (iow, I do not think of a use pattern for
> future users where being able to do some custom things between steps
> (1) and (2) would be useful), which is this function is about. (3)
> is what the user of this API would do, but shouldn't (4) be exported
> at the same time, if we are making (1+2) as an external API?
Good point.
It _also_ hints at the fact that we're not releasing the memory properly
after running the diffstat in the built-in `add -i`.
Will fix,
Dscho
>
> > void diff_addremove(struct diff_options *options,
> > int addremove, unsigned mode,
> > const struct object_id *oid,
> > diff --git a/diff.h b/diff.h
> > index b680b377b2..34fc658946 100644
> > --- a/diff.h
> > +++ b/diff.h
> > @@ -244,6 +244,22 @@ void diff_emit_submodule_error(struct diff_options *o, const char *err);
> > void diff_emit_submodule_pipethrough(struct diff_options *o,
> > const char *line, int len);
> >
> > +struct diffstat_t {
> > + int nr;
> > + int alloc;
> > + struct diffstat_file {
> > + char *from_name;
> > + char *name;
> > + char *print_name;
> > + const char *comments;
> > + unsigned is_unmerged:1;
> > + unsigned is_binary:1;
> > + unsigned is_renamed:1;
> > + unsigned is_interesting:1;
> > + uintmax_t added, deleted;
> > + } **files;
> > +};
> > +
> > enum color_diff {
> > DIFF_RESET = 0,
> > DIFF_CONTEXT = 1,
> > @@ -333,6 +349,9 @@ void diff_change(struct diff_options *,
> >
> > struct diff_filepair *diff_unmerge(struct diff_options *, const char *path);
> >
> > +void compute_diffstat(struct diff_options *options, struct diffstat_t *diffstat,
> > + struct diff_queue_struct *q);
> > +
> > #define DIFF_SETUP_REVERSE 1
> > #define DIFF_SETUP_USE_SIZE_CACHE 4
>
