threads / patch / 4713

patch, 4 partsmake filepairs detachable

Subject: [PATCH 2/4] make filepairs detachable

## tl;dr

5 messages between Jun 30, 2006 and Jun 30, 2006. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Alex Riesen· Jun 30, 2006, 00:27 UTC · lore

Actually, just make sure diff_flush does not crash for diff queue entries which were cleared.

---
 diff.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Show changes to diff.c +4 −0
diff --git a/diff.c b/diff.c
index 5a71489..b3480f6 100644
--- a/diff.c
+++ b/diff.c
@@ -2281,6 +2281,8 @@ void diff_flush(struct diff_options *opt
 	}
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
+		if (!p)
+			continue;
 		flush_one_pair(p, diff_output_format, options, diffstat);
 	}
 
@@ -2290,6 +2292,8 @@ void diff_flush(struct diff_options *opt
 	}
 
 	for (i = 0; i < q->nr; i++) {
+		if (!q->queue[i])
+		    continue;
 		if (diffstat && options->summary)
 			diff_summary(q->queue[i]);
 		diff_free_filepair(q->queue[i]);
-- 
1.4.1.rc1.g17dc
Junio C Hamano· Jun 30, 2006, 01:16 UTC · re: Alex Riesen · lore

Re: [PATCH 2/4] make filepairs detachable

fork0@t-online.de (Alex Riesen) writes:
> Actually, just make sure diff_flush does not crash for diff queue
> entries which were cleared.

Somehow I really feel uneasy about this one. I think it is the responsibility of the one who mangles the queue to adjust the entries and count consistent.

Alex Riesen· Jun 30, 2006, 07:42 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] make filepairs detachable

On 6/30/06, Junio C Hamano <junkio@cox.net> wrote:
Show 8 quoted lines
>
> > Actually, just make sure diff_flush does not crash for diff queue
> > entries which were cleared.
>
> Somehow I really feel uneasy about this one.  I think it is the
> responsibility of the one who mangles the queue to adjust the
> entries and count consistent.
>

That's what I get after being exposed to double-linked lists for too long: I begin to think it's fine to take whatever I want. The pairs are actually exactly what I need, so I take them. I think I just have to encapsulate it nicely, i.e.:

    struct diff_filepair *diff_queued_detach(int pos)
    {
        struct diff_filepair *pair = NULL;
        if (pos < diff_queued_diff.nr) {
            diff_queued_diff.queue[pos];
            if (diff_queued_diff.nr - pos > 1)
                memmove(pair->queue + pos, pair->queue + pos + 1,
                        diff_queued_diff.nr - pos - 1);
            diff_queued_diff.nr--;
        }
        return pair;
    }

BTW, is there any chance we get the struct diff_filepair's double-linked? They don't seem to be sorted. Will increase the memory footprint, though.

Junio C Hamano· Jun 30, 2006, 07:50 UTC · re: Alex Riesen · lore

Re: [PATCH 2/4] make filepairs detachable

"Alex Riesen" <raa.lkml@gmail.com> writes:
> BTW, is there any chance we get the struct diff_filepair's
> double-linked? They don't seem to be sorted. Will increase
> the memory footprint, though.

You do not necessarily have to do things in place. In fact, all of the standard diffcore transformers create a new queue, read from the old queue and place the filtered or munged result in the new queue, swap the global queued_diff variable to point at the new one and discard the old queue.

See diff.c::diffcore_apply_filter() for the simplest example.
Alex Riesen· Jun 30, 2006, 08:21 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/4] make filepairs detachable

On 6/30/06, Junio C Hamano <junkio@cox.net> wrote:
Show 13 quoted lines
>
> > BTW, is there any chance we get the struct diff_filepair's
> > double-linked? They don't seem to be sorted. Will increase
> > the memory footprint, though.
>
> You do not necessarily have to do things in place.  In fact, all
> of the standard diffcore transformers create a new queue, read
> from the old queue and place the filtered or munged result in
> the new queue, swap the global queued_diff variable to point at
> the new one and discard the old queue.
>
> See diff.c::diffcore_apply_filter() for the simplest example.
>
I see. Looks dangerous outside of diff.c though.

← back to recent threads