From 7054b5a2e3b2ce7f5b2e868e91d54b5ec0773089 Mon Sep 17 00:00:00 2001 From: huangs Date: Tue, 26 Jul 2016 14:46:13 -0700 Subject: [PATCH] [Courgette] Refactor BSDiff namespaces and bsdiff::search() interface. Details: - Move BSDiff (but not PagedArray) from namespace courgette to bsdiff. - Change namespace courgette::qsuf to qsuf. - Change bsdiff:search() to return struct {pos, size} so we don't need awkward pointer passing; update callers. - Updated BSDiff callers. Also fix weird hybrid usage by setup_util.cc, which calls Courgette's BSDiff, but using OK from BSPatch. BUG=608885 Review-Url: https://codereview.chromium.org/2031193002 Cr-Commit-Position: refs/heads/master@{#407924} --- chrome/installer/setup/setup_util.cc | 5 +- .../utility/chrome_content_utility_client.cc | 6 +- .../component_patcher_operation.cc | 6 +- courgette/bsdiff_memory_unittest.cc | 9 +-- courgette/courgette_tool.cc | 24 ++++--- courgette/simple_delta.cc | 10 +-- courgette/third_party/bsdiff/README.chromium | 8 ++- courgette/third_party/bsdiff/bsdiff.h | 23 ++++--- courgette/third_party/bsdiff/bsdiff_apply.cc | 14 +++- courgette/third_party/bsdiff/bsdiff_create.cc | 43 ++++++++----- courgette/third_party/bsdiff/bsdiff_search.h | 64 +++++++++---------- .../bsdiff/bsdiff_search_unittest.cc | 44 ++++++------- courgette/third_party/bsdiff/qsufsort.h | 5 +- .../third_party/bsdiff/qsufsort_unittest.cc | 51 +++++++++------ courgette/versioning_unittest.cc | 4 +- 15 files changed, 176 insertions(+), 140 deletions(-) diff --git a/chrome/installer/setup/setup_util.cc b/chrome/installer/setup/setup_util.cc index b86760525be4e4..572c54a3c73e0a 100644 --- a/chrome/installer/setup/setup_util.cc +++ b/chrome/installer/setup/setup_util.cc @@ -41,7 +41,6 @@ #include "chrome/installer/util/util_constants.h" #include "courgette/courgette.h" #include "courgette/third_party/bsdiff/bsdiff.h" -#include "third_party/bspatch/mbspatch.h" namespace installer { @@ -149,8 +148,8 @@ int BsdiffPatchFiles(const base::FilePath& src, if (src.empty() || patch.empty() || dest.empty()) return installer::PATCH_INVALID_ARGUMENTS; - const int patch_status = courgette::ApplyBinaryPatch(src, patch, dest); - const int exit_code = patch_status != OK ? + const int patch_status = bsdiff::ApplyBinaryPatch(src, patch, dest); + const int exit_code = patch_status != bsdiff::OK ? patch_status + kBsdiffErrorOffset : 0; LOG_IF(ERROR, exit_code) diff --git a/chrome/utility/chrome_content_utility_client.cc b/chrome/utility/chrome_content_utility_client.cc index 78038e406a6149..4a8f89494a9ceb 100644 --- a/chrome/utility/chrome_content_utility_client.cc +++ b/chrome/utility/chrome_content_utility_client.cc @@ -266,9 +266,9 @@ void ChromeContentUtilityClient::OnPatchFileBsdiff( if (input_file.empty() || patch_file.empty() || output_file.empty()) { Send(new ChromeUtilityHostMsg_PatchFile_Finished(-1)); } else { - const int patch_status = courgette::ApplyBinaryPatch(input_file, - patch_file, - output_file); + const int patch_status = bsdiff::ApplyBinaryPatch(input_file, + patch_file, + output_file); Send(new ChromeUtilityHostMsg_PatchFile_Finished(patch_status)); } ReleaseProcessIfNeeded(); diff --git a/components/update_client/component_patcher_operation.cc b/components/update_client/component_patcher_operation.cc index 2da622daacbcec..956bc0214465c1 100644 --- a/components/update_client/component_patcher_operation.cc +++ b/components/update_client/component_patcher_operation.cc @@ -208,8 +208,8 @@ void DeltaUpdateOpPatch::DoRun(const ComponentUnpacker::Callback& callback) { if (operation_ == kBsdiff) { DonePatching(callback, - courgette::ApplyBinaryPatch(input_abs_path_, patch_abs_path_, - output_abs_path_)); + bsdiff::ApplyBinaryPatch(input_abs_path_, patch_abs_path_, + output_abs_path_)); } else if (operation_ == kCourgette) { DonePatching(callback, courgette::ApplyEnsemblePatch( input_abs_path_.value().c_str(), @@ -224,7 +224,7 @@ void DeltaUpdateOpPatch::DonePatching( const ComponentUnpacker::Callback& callback, int result) { if (operation_ == kBsdiff) { - if (result == courgette::OK) { + if (result == bsdiff::OK) { callback.Run(ComponentUnpacker::kNone, 0); } else { callback.Run(ComponentUnpacker::kDeltaOperationFailure, diff --git a/courgette/bsdiff_memory_unittest.cc b/courgette/bsdiff_memory_unittest.cc index a9423d4e3c3b4e..09bfd2b810054d 100644 --- a/courgette/bsdiff_memory_unittest.cc +++ b/courgette/bsdiff_memory_unittest.cc @@ -25,8 +25,9 @@ void BSDiffMemoryTest::GenerateAndTestPatch(const std::string& old_text, new1.Init(new_text.c_str(), new_text.length()); courgette::SinkStream patch1; - courgette::BSDiffStatus status = CreateBinaryPatch(&old1, &new1, &patch1); - EXPECT_EQ(courgette::OK, status); + bsdiff::BSDiffStatus status = + bsdiff::CreateBinaryPatch(&old1, &new1, &patch1); + EXPECT_EQ(bsdiff::OK, status); courgette::SourceStream old2; courgette::SourceStream patch2; @@ -34,8 +35,8 @@ void BSDiffMemoryTest::GenerateAndTestPatch(const std::string& old_text, patch2.Init(patch1); courgette::SinkStream new2; - status = ApplyBinaryPatch(&old2, &patch2, &new2); - EXPECT_EQ(courgette::OK, status); + status = bsdiff::ApplyBinaryPatch(&old2, &patch2, &new2); + EXPECT_EQ(bsdiff::OK, status); EXPECT_EQ(new_text.length(), new2.Length()); EXPECT_EQ(0, memcmp(new_text.c_str(), new2.Buffer(), new_text.length())); } diff --git a/courgette/courgette_tool.cc b/courgette/courgette_tool.cc index d667f91eb6b549..70b177b817c40b 100644 --- a/courgette/courgette_tool.cc +++ b/courgette/courgette_tool.cc @@ -277,9 +277,10 @@ void DisassembleAdjustDiff(const base::FilePath& model_file, old_source.Init(old_stream ? *old_stream : empty_sink); new_source.Init(new_stream ? *new_stream : empty_sink); courgette::SinkStream patch_stream; - courgette::BSDiffStatus status = - courgette::CreateBinaryPatch(&old_source, &new_source, &patch_stream); - if (status != courgette::OK) Problem("-xxx failed."); + bsdiff::BSDiffStatus status = + bsdiff::CreateBinaryPatch(&old_source, &new_source, &patch_stream); + if (status != bsdiff::OK) + Problem("-xxx failed."); std::string append = std::string("-") + base::IntToString(i); @@ -327,7 +328,8 @@ void GenerateEnsemblePatch(const base::FilePath& old_file, courgette::Status status = courgette::GenerateEnsemblePatch(&old_stream, &new_stream, &patch_stream); - if (status != courgette::C_OK) Problem("-gen failed."); + if (status != courgette::C_OK) + Problem("-gen failed."); WriteSinkToFile(&patch_stream, patch_file); } @@ -398,10 +400,11 @@ void GenerateBSDiffPatch(const base::FilePath& old_file, new_stream.Init(new_buffer.data(), new_buffer.length()); courgette::SinkStream patch_stream; - courgette::BSDiffStatus status = - courgette::CreateBinaryPatch(&old_stream, &new_stream, &patch_stream); + bsdiff::BSDiffStatus status = + bsdiff::CreateBinaryPatch(&old_stream, &new_stream, &patch_stream); - if (status != courgette::OK) Problem("-genbsdiff failed."); + if (status != bsdiff::OK) + Problem("-genbsdiff failed."); WriteSinkToFile(&patch_stream, patch_file); } @@ -418,10 +421,11 @@ void ApplyBSDiffPatch(const base::FilePath& old_file, patch_stream.Init(patch_buffer.data(), patch_buffer.length()); courgette::SinkStream new_stream; - courgette::BSDiffStatus status = - courgette::ApplyBinaryPatch(&old_stream, &patch_stream, &new_stream); + bsdiff::BSDiffStatus status = + bsdiff::ApplyBinaryPatch(&old_stream, &patch_stream, &new_stream); - if (status != courgette::OK) Problem("-applybsdiff failed."); + if (status != bsdiff::OK) + Problem("-applybsdiff failed."); WriteSinkToFile(&new_stream, new_file); } diff --git a/courgette/simple_delta.cc b/courgette/simple_delta.cc index ed7eaa0ebceddd..d6f2cb459d5510 100644 --- a/courgette/simple_delta.cc +++ b/courgette/simple_delta.cc @@ -15,10 +15,10 @@ namespace courgette { namespace { -Status BSDiffStatusToStatus(BSDiffStatus status) { +Status BSDiffStatusToStatus(bsdiff::BSDiffStatus status) { switch (status) { - case OK: return C_OK; - case CRC_ERROR: return C_BINARY_DIFF_CRC_ERROR; + case bsdiff::OK: return C_OK; + case bsdiff::CRC_ERROR: return C_BINARY_DIFF_CRC_ERROR; default: return C_GENERAL_ERROR; } } @@ -27,14 +27,14 @@ Status BSDiffStatusToStatus(BSDiffStatus status) { Status ApplySimpleDelta(SourceStream* old, SourceStream* delta, SinkStream* target) { - return BSDiffStatusToStatus(ApplyBinaryPatch(old, delta, target)); + return BSDiffStatusToStatus(bsdiff::ApplyBinaryPatch(old, delta, target)); } Status GenerateSimpleDelta(SourceStream* old, SourceStream* target, SinkStream* delta) { VLOG(1) << "GenerateSimpleDelta " << old->Remaining() << " " << target->Remaining(); - return BSDiffStatusToStatus(CreateBinaryPatch(old, target, delta)); + return BSDiffStatusToStatus(bsdiff::CreateBinaryPatch(old, target, delta)); } } // namespace courgette diff --git a/courgette/third_party/bsdiff/README.chromium b/courgette/third_party/bsdiff/README.chromium index 489dd84347e36a..c87dbf8c206da5 100644 --- a/courgette/third_party/bsdiff/README.chromium +++ b/courgette/third_party/bsdiff/README.chromium @@ -19,13 +19,15 @@ incompatible with the original and it cannot be compiled outside the Chromium source tree or the Courgette project. List of changes made to original code: - - Wrapped functions in 'courgette' namespace. + - Wrapped functions in 'bsdiff' namespace. - Renamed .c files to .cc files. - - Added bsdiff.h header file. + - Added bsdiff.h and bsdiff_search.h header files. - Changed the code to use streams.h from courgette. - Changed the encoding of numbers to use the 'varint' encoding. - Reformatted code to be closer to Google coding standards. - Renamed variables. - Added comments. - - Extracted qsufsort into qsufsort.h in 'courgette::qsuf' namespace. + - Extracted qsufsort into qsufsort.h in 'qsuf' namespace. - Added unit tests for qsufsort. + - Fixed qsufsort pivoting issue: http://crbug.com/605565. + - Fixed search() comparison issue: http://crbug.com/620867. diff --git a/courgette/third_party/bsdiff/bsdiff.h b/courgette/third_party/bsdiff/bsdiff.h index 487300e4dd2d4f..a5da4ec9dd130d 100644 --- a/courgette/third_party/bsdiff/bsdiff.h +++ b/courgette/third_party/bsdiff/bsdiff.h @@ -47,6 +47,11 @@ #include "base/files/file_util.h" namespace courgette { +class SourceStream; +class SinkStream; +} // namespace courgette + +namespace bsdiff { enum BSDiffStatus { OK = 0, @@ -57,22 +62,19 @@ enum BSDiffStatus { WRITE_ERROR = 5 }; -class SourceStream; -class SinkStream; - // Creates a binary patch. // -BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, - SourceStream* new_stream, - SinkStream* patch_stream); +BSDiffStatus CreateBinaryPatch(courgette::SourceStream* old_stream, + courgette::SourceStream* new_stream, + courgette::SinkStream* patch_stream); // Applies the given patch file to a given source file. This method validates // the CRC of the original file stored in the patch file, before applying the // patch to it. // -BSDiffStatus ApplyBinaryPatch(SourceStream* old_stream, - SourceStream* patch_stream, - SinkStream* new_stream); +BSDiffStatus ApplyBinaryPatch(courgette::SourceStream* old_stream, + courgette::SourceStream* patch_stream, + courgette::SinkStream* new_stream); // As above, but simply takes the file paths. BSDiffStatus ApplyBinaryPatch(const base::FilePath& old_stream, @@ -94,5 +96,6 @@ typedef struct MBSPatchHeader_ { // null at end of string. #define MBS_PATCH_HEADER_TAG "GBSDIF42" -} // namespace +} // namespace bsdiff + #endif // COURGETTE_THIRD_PARTY_BSDIFF_BSDIFF_H_ diff --git a/courgette/third_party/bsdiff/bsdiff_apply.cc b/courgette/third_party/bsdiff/bsdiff_apply.cc index e0d99ad1c5d48e..9d96c62a77d417 100644 --- a/courgette/third_party/bsdiff/bsdiff_apply.cc +++ b/courgette/third_party/bsdiff/bsdiff_apply.cc @@ -44,7 +44,17 @@ #include "courgette/crc.h" #include "courgette/streams.h" -namespace courgette { +namespace { + +using courgette::CalculateCrc; +using courgette::SinkStream; +using courgette::SinkStreamSet; +using courgette::SourceStream; +using courgette::SourceStreamSet; + +} // namespace + +namespace bsdiff { BSDiffStatus MBS_ReadHeader(SourceStream* stream, MBSPatchHeader* header) { if (!stream->Read(header->tag, sizeof(header->tag))) @@ -215,4 +225,4 @@ BSDiffStatus ApplyBinaryPatch(const base::FilePath& old_file_path, return OK; } -} // namespace +} // namespace bsdiff diff --git a/courgette/third_party/bsdiff/bsdiff_create.cc b/courgette/third_party/bsdiff/bsdiff_create.cc index b89c2f1a57a997..4b7aeade2b6a6d 100644 --- a/courgette/third_party/bsdiff/bsdiff_create.cc +++ b/courgette/third_party/bsdiff/bsdiff_create.cc @@ -65,7 +65,18 @@ #include "courgette/third_party/bsdiff/paged_array.h" #include "courgette/third_party/bsdiff/qsufsort.h" -namespace courgette { +namespace { + +using courgette::CalculateCrc; +using courgette::PagedArray; +using courgette::SinkStream; +using courgette::SinkStreamSet; +using courgette::SourceStream; +using courgette::SourceStreamSet; + +} // namespace + +namespace bsdiff { static CheckBool WriteHeader(SinkStream* stream, MBSPatchHeader* header) { bool ok = stream->Write(header->tag, sizeof(header->tag)); @@ -165,26 +176,26 @@ BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, int lastscan = 0, lastpos = 0, lastoffset = 0; int scan = 0; - int match_length = 0; + SearchResult match(0, 0); while (scan < newsize) { - int pos = 0; int oldscore = 0; // Count of how many bytes of the current match at |scan| // extend the match at |lastscan|. + match.pos = 0; - scan += match_length; + scan += match.size; for (int scsc = scan; scan < newsize; ++scan) { - match_length = courgette::search&>( - I, old, oldsize, newbuf + scan, newsize - scan, &pos); + match = search&>( + I, old, oldsize, newbuf + scan, newsize - scan); - for (; scsc < scan + match_length; scsc++) + for (; scsc < scan + match.size; scsc++) if ((scsc + lastoffset < oldsize) && (old[scsc + lastoffset] == newbuf[scsc])) oldscore++; - if ((match_length == oldscore) && (match_length != 0)) + if ((match.size == oldscore) && (match.size != 0)) break; // Good continuing match, case (1) - if (match_length > oldscore + 8) + if (match.size > oldscore + 8) break; // New seed match, case (2) if ((scan + lastoffset < oldsize) && @@ -193,7 +204,7 @@ BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, // Case (3) continues in this loop until we fall out of the loop (4). } - if ((match_length != oldscore) || (scan == newsize)) { // Cases (2) and (4) + if ((match.size != oldscore) || (scan == newsize)) { // Cases (2) and (4) // This next chunk of code finds the boundary between the bytes to be // copied as part of the current triple, and the bytes to be copied as // part of the next triple. The |lastscan| match is extended forwards as @@ -206,8 +217,8 @@ BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, int lenb = 0; if (scan < newsize) { // i.e. not case (4); there is a match to extend. int score = 0, Sb = 0; - for (int i = 1; (scan >= lastscan + i) && (pos >= i); i++) { - if (old[pos - i] == newbuf[scan - i]) + for (int i = 1; (scan >= lastscan + i) && (match.pos >= i); i++) { + if (old[match.pos - i] == newbuf[scan - i]) score++; if (score * 2 - i > Sb * 2 - lenb) { Sb = score; @@ -247,7 +258,7 @@ BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, old[lastpos + lenf - overlap + i]) { score++; } - if (newbuf[scan - lenb + i] == old[pos - lenb + i]) { + if (newbuf[scan - lenb + i] == old[match.pos - lenb + i]) { score--; } if (score > Ss) { @@ -284,7 +295,7 @@ BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, uint32_t copy_count = lenf; uint32_t extra_count = gap; - int32_t seek_adjustment = ((pos - lenb) - (lastpos + lenf)); + int32_t seek_adjustment = ((match.pos - lenb) - (lastpos + lenf)); if (!control_stream_copy_counts->WriteVarint32(copy_count) || !control_stream_extra_counts->WriteVarint32(extra_count) || @@ -300,7 +311,7 @@ BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, #endif lastscan = scan - lenb; // Include the backward extension in seed. - lastpos = pos - lenb; // ditto. + lastpos = match.pos - lenb; // ditto. lastoffset = lastpos - lastscan; } } @@ -339,4 +350,4 @@ BSDiffStatus CreateBinaryPatch(SourceStream* old_stream, return OK; } -} // namespace courgette +} // namespace bsdiff diff --git a/courgette/third_party/bsdiff/bsdiff_search.h b/courgette/third_party/bsdiff/bsdiff_search.h index e4dd1e7e5f224a..78eee53073fd5f 100644 --- a/courgette/third_party/bsdiff/bsdiff_search.h +++ b/courgette/third_party/bsdiff/bsdiff_search.h @@ -33,8 +33,9 @@ // --Samuel Huang // 2015-08-19 - Optimized search() to be non-recursive. // --Samuel Huang -// 2016-06-17 - Moved matchlen() and search() to a new file; format; changed +// 2016-06-28 - Moved matchlen() and search() to a new file; format; changed // search() use std::lexicographical_compare(). +// 2016-06-30 - Changed matchlen() input; changed search() to return struct. // --Samuel Huang // Copyright 2016 The Chromium Authors. All rights reserved. @@ -45,57 +46,52 @@ #define COURGETTE_THIRD_PARTY_BSDIFF_BSDIFF_SEARCH_H_ #include -#include -namespace courgette { +namespace bsdiff { -// Similar to ::memcmp(), but returns match length instead. -inline int matchlen(const unsigned char* old, - int oldsize, - const unsigned char* newbuf, - int newsize) { - int i; +// Return values of search(). +struct SearchResult { + SearchResult(int pos_in, int size_in) : pos(pos_in), size(size_in) {} + int pos; + int size; +}; - for (i = 0; (i < oldsize) && (i < newsize); i++) - if (old[i] != newbuf[i]) - break; - - return i; +// Similar to ::memcmp(), but assumes equal |size| and returns match length. +inline int matchlen(const unsigned char* buf1, + const unsigned char* buf2, + int size) { + for (int i = 0; i < size; ++i) + if (buf1[i] != buf2[i]) + return i; + return size; } -// Finds a suffix in |old| that has the longest common prefix with |newbuf|, +// Finds a suffix in |old| that has the longest common prefix with |keybuf|, // aided by suffix array |I| of |old|. Returns the match length, and writes to // |pos| a position of best match in |old|. If multiple such positions exist, // |pos| would take an arbitrary one. template -int search(T I, - const unsigned char* old, - int oldsize, - const unsigned char* newbuf, - int newsize, - int* pos) { +SearchResult search(T I, + const unsigned char* srcbuf, + int srcsize, + const unsigned char* keybuf, + int keysize) { int lo = 0; - int hi = oldsize; + int hi = srcsize; while (hi - lo >= 2) { int mid = (lo + hi) / 2; - if (std::lexicographical_compare(old + I[mid], old + oldsize, newbuf, - newbuf + newsize)) { + if (std::lexicographical_compare( + srcbuf + I[mid], srcbuf + srcsize, keybuf, keybuf + keysize)) { lo = mid; } else { hi = mid; } } - - int x = matchlen(old + I[lo], oldsize - I[lo], newbuf, newsize); - int y = matchlen(old + I[hi], oldsize - I[hi], newbuf, newsize); - if (x > y) { - *pos = I[lo]; - return x; - } - *pos = I[hi]; - return y; + int x = matchlen(srcbuf + I[lo], keybuf, std::min(srcsize - I[lo], keysize)); + int y = matchlen(srcbuf + I[hi], keybuf, std::min(srcsize - I[hi], keysize)); + return (x > y) ? SearchResult(I[lo], x) : SearchResult(I[hi], y); } -} // namespace courgette +} // namespace bsdiff #endif // COURGETTE_THIRD_PARTY_BSDIFF_BSDIFF_SEARCH_H_ diff --git a/courgette/third_party/bsdiff/bsdiff_search_unittest.cc b/courgette/third_party/bsdiff/bsdiff_search_unittest.cc index dff7172dc5ce31..bcc5b2517179a2 100644 --- a/courgette/third_party/bsdiff/bsdiff_search_unittest.cc +++ b/courgette/third_party/bsdiff/bsdiff_search_unittest.cc @@ -20,11 +20,11 @@ TEST(BSDiffSearchTest, Search) { const unsigned char* buf = reinterpret_cast(str); std::vector I(size + 1); std::vector V(size + 1); - courgette::qsuf::qsufsort(&I[0], &V[0], buf, size); + qsuf::qsufsort(&I[0], &V[0], buf, size); // Specific queries. const struct { - int exp_pos; // -1 means "don't care". + int exp_match_pos; // -1 means "don't care". int exp_match_size; const char* query_str; } test_cases[] = { @@ -44,7 +44,7 @@ TEST(BSDiffSearchTest, Search) { // Exact and non-unique match: take lexicographical first. {-1, 3, "the"}, // Non-unique prefix. {-1, 1, " "}, - // Partial and non-unique match: no guarantees on |pos|! + // Partial and non-unique match: no guarantees on |match.pos|! {-1, 4, "the apple"}, // query < "the l"... < "the q"... {-1, 4, "the opera"}, // "the l"... < query < "the q"... {-1, 4, "the zebra"}, // "the l"... < "the q"... < query @@ -64,22 +64,21 @@ TEST(BSDiffSearchTest, Search) { reinterpret_cast(test_case.query_str); // Perform the search. - int pos = 0; - int match_size = - courgette::search(&I[0], buf, size, query_buf, query_size, &pos); + bsdiff::SearchResult match = + bsdiff::search(&I[0], buf, size, query_buf, query_size); // Check basic properties and match with expected values. - EXPECT_GE(match_size, 0); - EXPECT_LE(match_size, query_size); - if (match_size > 0) { - EXPECT_GE(pos, 0); - EXPECT_LE(pos, size - match_size); - EXPECT_EQ(0, ::memcmp(buf + pos, query_buf, match_size)); + EXPECT_GE(match.size, 0); + EXPECT_LE(match.size, query_size); + if (match.size > 0) { + EXPECT_GE(match.pos, 0); + EXPECT_LE(match.pos, size - match.size); + EXPECT_EQ(0, ::memcmp(buf + match.pos, query_buf, match.size)); } - if (test_case.exp_pos >= 0) { - EXPECT_EQ(test_case.exp_pos, pos); + if (test_case.exp_match_pos >= 0) { + EXPECT_EQ(test_case.exp_match_pos, match.pos); } - EXPECT_EQ(test_case.exp_match_size, match_size); + EXPECT_EQ(test_case.exp_match_size, match.size); } } @@ -103,7 +102,7 @@ TEST(BSDiffSearchTest, SearchExact) { reinterpret_cast(test_cases[idx]); std::vector I(size + 1); std::vector V(size + 1); - courgette::qsuf::qsufsort(&I[0], &V[0], buf, size); + qsuf::qsufsort(&I[0], &V[0], buf, size); // Test exact matches for every non-empty substring. for (int lo = 0; lo < size; ++lo) { @@ -113,14 +112,13 @@ TEST(BSDiffSearchTest, SearchExact) { ASSERT_EQ(query_size, hi - lo); const unsigned char* query_buf = reinterpret_cast(query.c_str()); - int pos = 0; - int match_size = - courgette::search(&I[0], buf, size, query_buf, query_size, &pos); + bsdiff::SearchResult match = + bsdiff::search(&I[0], buf, size, query_buf, query_size); - EXPECT_EQ(query_size, match_size); - EXPECT_GE(pos, 0); - EXPECT_LE(pos, size - match_size); - std::string suffix(buf + pos, buf + size); + EXPECT_EQ(query_size, match.size); + EXPECT_GE(match.pos, 0); + EXPECT_LE(match.pos, size - match.size); + std::string suffix(buf + match.pos, buf + size); EXPECT_EQ(suffix.substr(0, query_size), query); } } diff --git a/courgette/third_party/bsdiff/qsufsort.h b/courgette/third_party/bsdiff/qsufsort.h index 323c1d1404271e..dae49d4116a21a 100644 --- a/courgette/third_party/bsdiff/qsufsort.h +++ b/courgette/third_party/bsdiff/qsufsort.h @@ -48,7 +48,6 @@ #ifndef COURGETTE_THIRD_PARTY_BSDIFF_QSUFSORT_H_ #define COURGETTE_THIRD_PARTY_BSDIFF_QSUFSORT_H_ -namespace courgette { namespace qsuf { // ------------------------------------------------------------------------ @@ -59,7 +58,8 @@ namespace qsuf { // (2) indentation and spacing, // (3) using 'const', // (4) changing the V and I parameters from int* to template . -// (5) optimizing split() and search(); fix styles. +// (5) optimizing split(); fix styles. +// (6) moving matchlen() and search() to a separate file. // // The code appears to be a rewritten version of the suffix array algorithm // presented in "Faster Suffix Sorting" by N. Jesper Larsson and Kunihiko @@ -221,6 +221,5 @@ static void qsufsort(T I, T V, const unsigned char* old, int oldsize) { // ------------------------------------------------------------------------ } // namespace qsuf -} // namespace courgette #endif // COURGETTE_THIRD_PARTY_BSDIFF_QSUFSORT_H_ diff --git a/courgette/third_party/bsdiff/qsufsort_unittest.cc b/courgette/third_party/bsdiff/qsufsort_unittest.cc index 31cb720bd264e5..5941e5f68c8f40 100644 --- a/courgette/third_party/bsdiff/qsufsort_unittest.cc +++ b/courgette/third_party/bsdiff/qsufsort_unittest.cc @@ -4,16 +4,27 @@ #include "courgette/third_party/bsdiff/qsufsort.h" -#include - #include +#include #include -#include #include #include "base/macros.h" #include "testing/gtest/include/gtest/gtest.h" +namespace { + +bool IsPermutation(const std::vector v) { + std::vector v_sorted(v); + std::sort(v_sorted.begin(), v_sorted.end()); + for (int i = 0; i < static_cast(v.size()); ++i) + if (i != v_sorted[i]) + return false; + return true; +} + +} // namespace + TEST(QSufSortTest, Sort) { const char* test_cases[] = { "", @@ -31,29 +42,31 @@ TEST(QSufSortTest, Sort) { }; for (size_t idx = 0; idx < arraysize(test_cases); ++idx) { - int len = static_cast(::strlen(test_cases[idx])); - const unsigned char* s = + int size = static_cast(::strlen(test_cases[idx])); + const unsigned char* buf = reinterpret_cast(test_cases[idx]); // Generate the suffix array as I. - std::vector I(len + 1); - std::vector V(len + 1); - courgette::qsuf::qsufsort(&I[0], &V[0], s, len); + std::vector I(size + 1); + std::vector V(size + 1); + qsuf::qsufsort(&I[0], &V[0], buf, size); + + // Expect I[] and V[] to be a permutation of [0, size]. + EXPECT_TRUE(IsPermutation(I)); + EXPECT_TRUE(IsPermutation(V)); - // Expect that I[] is a permutation of [0, len]. - std::vector I_sorted(I); - std::sort(I_sorted.begin(), I_sorted.end()); - for (int i = 0; i < len + 1; ++i) - EXPECT_EQ(i, I_sorted[i]); + // Expect V[] to be inverse of I[]. + for (int i = 0; i < size + 1; ++i) + EXPECT_EQ(i, V[I[i]]); // First string must be empty string. - EXPECT_EQ(len, I[0]); + EXPECT_EQ(size, I[0]); - // Expect that the |len + 1| suffixes are strictly ordered. - const unsigned char* end = s + len; - for (int i = 0; i < len; ++i) { - const unsigned char* suf1 = s + I[i]; - const unsigned char* suf2 = s + I[i + 1]; + // Expect that the |size + 1| suffixes are strictly ordered. + const unsigned char* end = buf + size; + for (int i = 0; i < size; ++i) { + const unsigned char* suf1 = buf + I[i]; + const unsigned char* suf2 = buf + I[i + 1]; bool is_less = std::lexicographical_compare(suf1, end, suf2, end); EXPECT_TRUE(is_less); } diff --git a/courgette/versioning_unittest.cc b/courgette/versioning_unittest.cc index 3a3a4cbbaa1fc6..0257ac46ff597c 100644 --- a/courgette/versioning_unittest.cc +++ b/courgette/versioning_unittest.cc @@ -34,10 +34,10 @@ void VersioningTest::TestApplyingOldBsDiffPatch( courgette::SinkStream generated_stream; - courgette::BSDiffStatus status = courgette::ApplyBinaryPatch( + bsdiff::BSDiffStatus status = bsdiff::ApplyBinaryPatch( &old_stream, &patch_stream, &generated_stream); - EXPECT_EQ(status, courgette::OK); + EXPECT_EQ(status, bsdiff::OK); size_t expected_length = expected_buffer.size(); size_t generated_length = generated_stream.Length();