git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 0/4] line-log: be more careful when adjusting multiple line ranges

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Aug 4, 2018, 22:18 UTC
Message-ID
<pull.15.git.gitgitgadget@gmail.com>

I am a heavy user of git log -L .... In fact, I use the feature where multiple ranges can be specified extensively, via a not-exactly-trivial shell script function that takes the currently staged changes (or if none are staged, the current unstanged changes) and turns them into the corresponding commit history:

staged_log () { #
        diff="$(git diff --cached -U1)"
        test -n "$diff" ||
        diff="$(git diff -U1)"
        test -n "$diff" ||
        die "No changes"
        eval "git log $(echo "$diff" |
                sed -ne '/^--- a\//{s/^-* a\/\(.*\)/'\''\1'\''/;x}' -e \
                        '/^@@ -/{s/^@@ -\([^, ]*\),\([^ ]*\).*/-L \1,+\2/;G;s/\n/:/g;p}' |
                        tr '\n' ' ')"
}

This is an extremely useful way to look at the history, especially when trying to fix up a commit deep in a long branch (or a thicket of branches).

Today, however, this method failed me, by greeting me with an assertion. When I tried to paper over that assertion by joining line ranges that became adjacent (or overlapping), it still produced a segmentation fault when the line-log tried to print lines past the file contents.

So I had no choice but to fix this properly.

I still wanted to keep the optimization where multiple line ranges are joined into a single one (I am convinced that this also affects the output, where previously multiple hunks would have been displayed, but I ran out of time to investigate this). This is the 3rd patch. It is not purely an optimization, as the assertion would still trigger when line ranges could be joined.

Now, I am fairly certain that the changes are correct, but given my track record with off-by-one bugs (and once even an off-by-two bug), I would really appreciate some thorough review of this code, in particular the second one that is the actual bug fix. I am specifically interested in reviews from people who know line-log.c pretty well and can tell me whether the src[i].end > target[j].end is correct, or whether it should actually have been a >= (I tried to wrap my head around this, but I would feel more comfortable if a domain expert would analyze this, whistling, and looking Eric's way).

Cc: Eric Sunshine sunshine@sunshineco.com [sunshine@sunshineco.com]
Johannes Schindelin (4):
  line-log: demonstrate a bug with nearly-overlapping ranges
  line-log: adjust start/end of ranges individually
  line-log: optimize ranges by joining them when possible
  line-log: convert an assertion to a full BUG() call
 line-log.c          | 18 +++++++++++++++---
 t/t4211-line-log.sh | 17 +++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)
base-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-15%2Fdscho%2Fline-log-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-15/dscho/line-log-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/15
-- 
gitgitgadget
Next: Johannes Schindelin via GitGitGadget
Message 1 of 20 in “line-log: be more careful when adjusting multiple line ranges”
  1. 0/4 line-log: be more careful when adjusting multiple line rangesJohannes Schindelin via GitGitGadget, Aug 4, 2018
  2. 1/4 line-log: demonstrate a bug with nearly-overlapping rangesJohannes Schindelin via GitGitGadget, Aug 4, 2018
  3. Jonathan NiederAug 5, 2018
  4. Johannes SchindelinAug 6, 2018
  5. Jonathan NiederAug 6, 2018
  6. Jonathan NiederAug 6, 2018
  7. 2/4 line-log: adjust start/end of ranges individuallyJohannes Schindelin via GitGitGadget, Aug 4, 2018
  8. Eric SunshineAug 5, 2018
  9. Eric SunshineAug 5, 2018
  10. Johannes SchindelinAug 6, 2018
  11. 3/4 line-log: optimize ranges by joining them when possibleJohannes Schindelin via GitGitGadget, Aug 4, 2018
  12. Junio C HamanoAug 5, 2018
  13. Andrei RybakAug 5, 2018
  14. Eric SunshineAug 5, 2018
  15. 4/4 line-log: convert an assertion to a full BUG() callJohannes Schindelin via GitGitGadget, Aug 4, 2018
  16. Eric SunshineAug 5, 2018
  17. Johannes SchindelinAug 6, 2018
  18. Eric SunshineAug 7, 2018
  19. Eric SunshineAug 7, 2018
  20. Eric SunshineAug 5, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.