gitk: Fix possible infinite loop and display corruption

paulusmack · paulusmack · commit 52b8ea934ecd · 2009-03-02T09:38:17.000+11:00
This fixes an issue reported by Johannes Sixt on the git mailing list:

&gt; This recipe sends gitk into an endless loop. In git.git do:
&gt;
&gt; cd t
&gt; # remove chmod a+x A near the end of the file
&gt; sed -i 's/chmod/: chmod/' t3400-rebase.sh
&gt; sh t3400-rebase.sh --debug
&gt; cd trash\ directory.t3400-rebase/
&gt; gitk master modechange modechange@{1}
&gt;
&gt;
&gt; I briefly see the history chart, but the dot that should be modechange@{1}
&gt; is missing. One automatically selected commit is shown in the diff section
&gt; below. But then the commit list is cleared and gitk goes into an infinite
&gt; loop.
&gt;
&gt; Things work alright if either modechange@{1} is dropped, or the 'chmod'
&gt; line is left unchanged, which is a bit strange.
&gt;
&gt; This is with git version 1.6.1.2.390.gba743

There were actually two problems.  This recipe created a situation where
git log would output a child commit after its parent.  This meant that
we called fix_reversal which called splitvarc, which should call modify_arc
to note the fact that it has modified the arc that it has just split.  It
wasn't, which meant that displayorder and other variables got into an
inconsistent state (a commit appearing twice in displayorder).

This then meant that the targetrow/targetid logic in drawvisible thought
it need to redraw each time.  That, together with the fact that drawvisible
called drawcommits which called drawvisible if a redraw was needed, led
to the infinite loop.

In fact drawvisible is now the only caller of drawcommits.  Thus, the
start and end row arguments to drawcommits always encompass the whole
visible area, so drawcommits doesn't need to call drawvisible to redraw;
it just needs to clear the screen and draw what it's been asked to.

This fixes these two problems by adding a call to modify_arc in
splitvarc and by taking out the call to drawvisible in drawcommits.
It also removes an unrelated left-over debugging puts in external_blame.

Signed-off-by: Paul Mackerras &lt;paulus@samba.org&gt;
diff --git a/gitk b/gitk
@@ -701,16 +701,17 @@ proc newvarc {view id} {
 }
 
 proc splitvarc {p v} {
-    global varcid varcstart varccommits varctok
+    global varcid varcstart varccommits varctok vtokmod
     global vupptr vdownptr vleftptr vbackptr varcix varcrow vlastins
 
     set oa $varcid($v,$p)
+    set otok [lindex $varctok($v) $oa]
     set ac $varccommits($v,$oa)
     set i [lsearch -exact $varccommits($v,$oa) $p]
     if {$i <= 0} return
     set na [llength $varctok($v)]
     # "%" sorts before "0"...
-    set tok "[lindex $varctok($v) $oa]%[strrep $i]"
+    set tok "$otok%[strrep $i]"
     lappend varctok($v) $tok
     lappend varcrow($v) {}
     lappend varcix($v) {}
@@ -730,6 +731,9 @@ proc splitvarc {p v} {
     for {set b [lindex $vdownptr($v) $na]} {$b != 0} {set b [lindex $vleftptr($v) $b]} {
 	lset vupptr($v) $b $na
     }
+    if {[string compare $otok $vtokmod($v)] <= 0} {
+	modify_arc $v $oa
+    }
 }
 
 proc renumbervarc {a v} {
@@ -3363,7 +3367,6 @@ proc external_blame {parent_idx {line {}}} {
     # being given an absolute path...
     set f [make_relative $f]
     lappend cmdline $base_commit $f
-    puts "cmdline={$cmdline}"
     if {[catch {eval exec $cmdline &} err]} {
 	error_popup "[mc "git gui blame: command failed:"] $err"
     }
@@ -5731,7 +5734,6 @@ proc drawcommits {row {endrow {}}} {
     optimize_rows $ro1 0 $r2
     if {$need_redisplay || $nrows_drawn > 2000} {
 	clear_display
-	drawvisible
     }
 
     # make the lines join to already-drawn rows either side
