Skip to content

Commit

Permalink
fix bug in original bsdiff program using memcmp for string compare
Browse files Browse the repository at this point in the history
  • Loading branch information
StefanKarpinski committed Feb 14, 2020
1 parent 9709c93 commit 3d23178
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
11 changes: 8 additions & 3 deletions src/BSDiff.jl
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ function match_length(
return l
end

@inline function strcmp(p::Ptr{UInt8}, m::Int, q::Ptr{UInt8}, n::Int)
x = Base._memcmp(p, q, min(m, n))
x == 0 ? cmp(m, n) : sign(x)
end

"""
Search for the longest prefix of new[ind:end] in old.
Uses the suffix array of old to search efficiently.
Expand All @@ -128,19 +133,19 @@ function prefix_search(
old_p = pointer(old)
new_p = pointer(new, ind)
# invariant: longest match is in suffixes[lo:hi]
lo, hi = 1, length(suffixes)
lo, hi = 1, old_n
while hi - lo 2
m = (lo + hi) >>> 1
s = suffixes[m]
if 0 < Base._memcmp(new_p, old_p + s, min(new_n, old_n - s))
if 0 < strcmp(new_p, new_n, old_p + s, old_n - s)
lo = m
else
hi = m
end
end
i = suffixes[lo]+1
m = match_length(old, i, new, ind)
lo == hi && return i, m
lo == hi && return (i, m)
j = suffixes[hi]+1
n = match_length(old, j, new, ind)
m > n ? (i, m) : (j, n)
Expand Down
5 changes: 4 additions & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ const test_data = artifact"test_data"
diff = sprint() do io
BSDiff.write_diff(io, old_data, new_data)
end |> codeunits
@test read(ref) == diff
# this test is unequal because the original bsdiff code is buggy:
# it uses `memcmp(old, new, min(length(old), length(new)))` whereas
# it should break memcmp ties by comparing the length of old & new
@test read(ref) diff
# test that applying reference patch to old produces new
new_data′ = open(ref) do patch
sprint() do new
Expand Down

0 comments on commit 3d23178

Please sign in to comment.