@matheusbsbs / matheus-bagliano / commits / 52b8ea934e
Fork 0

gitk: Fix possible infinite loop and display corruption

This fixes an issue reported by Johannes Sixt on the git mailing list: > This recipe sends gitk into an endless loop. In git.git do: > > cd t > # remove chmod a+x A near the end of the file > sed -i 's/chmod/: chmod/' t3400-rebase.sh > sh t3400-rebase.sh --debug > cd trash\ directory.t3400-rebase/ > gitk master modechange modechange@{1} > > > I briefly see the history chart, but the dot that should be modechange@{1} > is missing. One automatically selected commit is shown in the diff section > below. But then the commit list is cleared and gitk goes into an infinite > loop. > > Things work alright if either modechange@{1} is dropped, or the 'chmod' > line is left unchanged, which is a bit strange. > > 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 <paulus@samba.org>

Paul Mackerras committed Mar 2, 2009 at 09:38 UTC 52b8ea934ecd24b52806188b53367aaa6185deb3
1 file changed +6 -4
gitk
+6 -4
@@ -701,16 +701,17 @@ proc newvarc {view id} {
701 }
702
703 proc splitvarc {p v} {
704 - global varcid varcstart varccommits varctok
704 + global varcid varcstart varccommits varctok vtokmod
705 global vupptr vdownptr vleftptr vbackptr varcix varcrow vlastins
706
707 set oa $varcid($v,$p)
708 + set otok [lindex $varctok($v) $oa]
709 set ac $varccommits($v,$oa)
710 set i [lsearch -exact $varccommits($v,$oa) $p]
711 if {$i <= 0} return
712 set na [llength $varctok($v)]
713 # "%" sorts before "0"...
713 - set tok "[lindex $varctok($v) $oa]%[strrep $i]"
714 + set tok "$otok%[strrep $i]"
715 lappend varctok($v) $tok
716 lappend varcrow($v) {}
717 lappend varcix($v) {}
@@ -730,6 +731,9 @@ proc splitvarc {p v} {
731 for {set b [lindex $vdownptr($v) $na]} {$b != 0} {set b [lindex $vleftptr($v) $b]} {
732 lset vupptr($v) $b $na
733 }
734 + if {[string compare $otok $vtokmod($v)] <= 0} {
735 + modify_arc $v $oa
736 + }
737 }
738
739 proc renumbervarc {a v} {
@@ -3363,7 +3367,6 @@ proc external_blame {parent_idx {line {}}} {
3367 # being given an absolute path...
3368 set f [make_relative $f]
3369 lappend cmdline $base_commit $f
3366 - puts "cmdline={$cmdline}"
3370 if {[catch {eval exec $cmdline &} err]} {
3371 error_popup "[mc "git gui blame: command failed:"] $err"
3372 }
@@ -5731,7 +5734,6 @@ proc drawcommits {row {endrow {}}} {
5734 optimize_rows $ro1 0 $r2
5735 if {$need_redisplay || $nrows_drawn > 2000} {
5736 clear_display
5734 - drawvisible
5737 }
5738
5739 # make the lines join to already-drawn rows either side