tequilaOS/platform_bootable_recovery
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Remove the assumption of target chunk size in imgdiff

Browse source 
In the split mode of imgdiff, we used to assume that the size of a split
target chunk is always greater than the blocksize i.e. 4096. This may
lead to the following assertion failure:
I0221 04:57:33.451323 818464 common.py:205 imgdiff F 02-21 04:57:33 821203 821203 imgdiff.cpp:999]
Check failed: tgt_size >= BLOCK_SIZE (tgt_size=476, BLOCK_SIZE=4096)

This CL removes the assumption and handles the edge cases.

Test: generate and verify the incremental update for TFs in the bug; unit test passes

Bug: 73757557
Bug: 73711365
Change-Id: Iadbb4ee658995f5856cd488f3793980881a59620
This commit is contained in:
Tianjie Xu 2018-02-22 15:40:39 -08:00
parent bf52b7e00b
commit 572abbb81c
3 changed files with 131 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
applypatch/imgdiff.cpp
View file

@ -955,14 +955,17 @@ bool ZipModeImage::SplitZipModeImageWithLimit(const ZipModeImage& tgt_image,
tgt->GetRawDataLength());
}
} else {
ZipModeImage::AddSplitImageFromChunkList(tgt_image, src_image, src_ranges, split_tgt_chunks,
split_src_chunks, split_tgt_images,
split_src_images);
bool added_image = ZipModeImage::AddSplitImageFromChunkList(
tgt_image, src_image, src_ranges, split_tgt_chunks, split_src_chunks, split_tgt_images,
split_src_images);
split_tgt_chunks.clear();
split_src_chunks.clear();
used_src_ranges.Insert(src_ranges);
split_src_ranges->push_back(std::move(src_ranges));
// No need to update the split_src_ranges if we don't update the split source images.
if (added_image) {
used_src_ranges.Insert(src_ranges);
split_src_ranges->push_back(std::move(src_ranges));
}
src_ranges.Clear();
// We don't have enough space for the current chunk; start a new split image and handle
@ -973,9 +976,12 @@ bool ZipModeImage::SplitZipModeImageWithLimit(const ZipModeImage& tgt_image,
// TODO Trim it in case the CD exceeds limit too much.
src_ranges.Insert(central_directory->GetStartOffset(), central_directory->DataLengthForPatch());
ZipModeImage::AddSplitImageFromChunkList(tgt_image, src_image, src_ranges, split_tgt_chunks,
split_src_chunks, split_tgt_images, split_src_images);
split_src_ranges->push_back(std::move(src_ranges));
bool added_image = ZipModeImage::AddSplitImageFromChunkList(tgt_image, src_image, src_ranges,
split_tgt_chunks, split_src_chunks,
split_tgt_images, split_src_images);
if (added_image) {
split_src_ranges->push_back(std::move(src_ranges));
}
ValidateSplitImages(*split_tgt_images, *split_src_images, *split_src_ranges,
tgt_image.file_content_.size());
@ -983,7 +989,7 @@ bool ZipModeImage::SplitZipModeImageWithLimit(const ZipModeImage& tgt_image,
return true;
}
void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
bool ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
const ZipModeImage& src_image,
const SortedRangeSet& split_src_ranges,
const std::vector<ImageChunk>& split_tgt_chunks,
@ -991,12 +997,6 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
std::vector<ZipModeImage>* split_tgt_images,
std::vector<ZipModeImage>* split_src_images) {
CHECK(!split_tgt_chunks.empty());
// Target chunks should occupy at least one block.
// TODO put a warning and change the type to raw if it happens in extremely rare cases.
size_t tgt_size = split_tgt_chunks.back().GetStartOffset() +
split_tgt_chunks.back().DataLengthForPatch() -
split_tgt_chunks.front().GetStartOffset();
CHECK_GE(tgt_size, BLOCK_SIZE);
std::vector<ImageChunk> aligned_tgt_chunks;
@ -1015,7 +1015,12 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
i++;
}
CHECK_LT(i, split_tgt_chunks.size());
// Nothing left after alignment in the current split tgt chunks; skip adding the split_tgt_image.
if (i == split_tgt_chunks.size()) {
return false;
}
aligned_tgt_chunks.insert(aligned_tgt_chunks.end(), split_tgt_chunks.begin() + i + 1,
split_tgt_chunks.end());
CHECK(!aligned_tgt_chunks.empty());
@ -1024,8 +1029,10 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
size_t end_offset =
aligned_tgt_chunks.back().GetStartOffset() + aligned_tgt_chunks.back().GetRawDataLength();
if (end_offset % BLOCK_SIZE != 0 && end_offset < tgt_image.file_content_.size()) {
size_t tail_block_length = std::min<size_t>(tgt_image.file_content_.size() - end_offset,
BLOCK_SIZE - (end_offset % BLOCK_SIZE));
aligned_tgt_chunks.emplace_back(CHUNK_NORMAL, end_offset, &tgt_image.file_content_,
BLOCK_SIZE - (end_offset % BLOCK_SIZE));
tail_block_length);
}
ZipModeImage split_tgt_image(false);
@ -1049,6 +1056,8 @@ void ZipModeImage::AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
split_tgt_images->push_back(std::move(split_tgt_image));
split_src_images->push_back(std::move(split_src_image));
return true;
}
void ZipModeImage::ValidateSplitImages(const std::vector<ZipModeImage>& split_tgt_images,
@ -1536,7 +1545,7 @@ int imgdiff(int argc, const char** argv) {
" patches together and output them into <patch-file>.\n"
" --split-info, Output the split information (patch_size, tgt_size, src_ranges);\n"
" zip mode with block-limit only.\n"
" --debug_dir, Debug directory to put the split srcs and patches, zip mode only.\n"
" --debug-dir, Debug directory to put the split srcs and patches, zip mode only.\n"
" -v, --verbose, Enable verbose logging.";
return 2;
}

5
applypatch/include/applypatch/imgdiff_image.h
View file

@ -265,8 +265,9 @@ class ZipModeImage : public Image {
std::vector<SortedRangeSet>& split_src_ranges,
size_t total_tgt_size);
// Construct the dummy split images based on the chunks info and source ranges; and move them into
// the given vectors.
static void AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
// the given vectors. Return true if we add a new split image into |split_tgt_images|, and
// false otherwise.
static bool AddSplitImageFromChunkList(const ZipModeImage& tgt_image,
const ZipModeImage& src_image,
const SortedRangeSet& split_src_ranges,
const std::vector<ImageChunk>& split_tgt_chunks,

101
tests/component/imgdiff_test.cpp
View file

@ -969,3 +969,104 @@ TEST(ImgdiffTest, zip_mode_large_enough_limit) {
// Expect 1 piece of patch since limit is larger than the zip file size.
GenerateAndCheckSplitTarget(debug_dir.path, 1, tgt);
}
TEST(ImgdiffTest, zip_mode_large_apk_small_target_chunk) {
TemporaryFile tgt_file;
FILE* tgt_file_ptr = fdopen(tgt_file.release(), "wb");
ZipWriter tgt_writer(tgt_file_ptr);
// The first entry is less than 4096 bytes, followed immediately by an entry that has a very
// large counterpart in the source file. Therefore the first entry will be patched separately.
std::string small_chunk("a", 2000);
ASSERT_EQ(0, tgt_writer.StartEntry("a", 0));
ASSERT_EQ(0, tgt_writer.WriteBytes(small_chunk.data(), small_chunk.size()));
ASSERT_EQ(0, tgt_writer.FinishEntry());
construct_store_entry(
{
{ "b", 12, 'b' }, { "c", 3, 'c' },
},
&tgt_writer);
ASSERT_EQ(0, tgt_writer.Finish());
ASSERT_EQ(0, fclose(tgt_file_ptr));
TemporaryFile src_file;
FILE* src_file_ptr = fdopen(src_file.release(), "wb");
ZipWriter src_writer(src_file_ptr);
construct_store_entry({ { "a", 1, 'a' }, { "b", 13, 'b' }, { "c", 1, 'c' } }, &src_writer);
ASSERT_EQ(0, src_writer.Finish());
ASSERT_EQ(0, fclose(src_file_ptr));
// Compute patch.
TemporaryFile patch_file;
TemporaryFile split_info_file;
TemporaryDir debug_dir;
std::string split_info_arg = android::base::StringPrintf("--split-info=%s", split_info_file.path);
std::string debug_dir_arg = android::base::StringPrintf("--debug-dir=%s", debug_dir.path);
std::vector<const char*> args = {
"imgdiff", "-z", "--block-limit=10", split_info_arg.c_str(), debug_dir_arg.c_str(),
src_file.path, tgt_file.path, patch_file.path,
};
ASSERT_EQ(0, imgdiff(args.size(), args.data()));
std::string tgt;
ASSERT_TRUE(android::base::ReadFileToString(tgt_file.path, &tgt));
// Expect three split src images:
// src_piece 0: a 1 blocks
// src_piece 1: b-0 10 blocks
// src_piece 2: b-1 3 blocks, c 1 blocks, CD
GenerateAndCheckSplitTarget(debug_dir.path, 3, tgt);
}
TEST(ImgdiffTest, zip_mode_large_apk_skipped_small_target_chunk) {
TemporaryFile tgt_file;
FILE* tgt_file_ptr = fdopen(tgt_file.release(), "wb");
ZipWriter tgt_writer(tgt_file_ptr);
construct_store_entry(
{
{ "a", 11, 'a' },
},
&tgt_writer);
// Construct a tiny target entry of 1 byte, which will be skipped due to the tail alignment of
// the previous entry.
std::string small_chunk("b", 1);
ASSERT_EQ(0, tgt_writer.StartEntry("b", 0));
ASSERT_EQ(0, tgt_writer.WriteBytes(small_chunk.data(), small_chunk.size()));
ASSERT_EQ(0, tgt_writer.FinishEntry());
ASSERT_EQ(0, tgt_writer.Finish());
ASSERT_EQ(0, fclose(tgt_file_ptr));
TemporaryFile src_file;
FILE* src_file_ptr = fdopen(src_file.release(), "wb");
ZipWriter src_writer(src_file_ptr);
construct_store_entry(
{
{ "a", 11, 'a' }, { "b", 11, 'b' },
},
&src_writer);
ASSERT_EQ(0, src_writer.Finish());
ASSERT_EQ(0, fclose(src_file_ptr));
// Compute patch.
TemporaryFile patch_file;
TemporaryFile split_info_file;
TemporaryDir debug_dir;
std::string split_info_arg = android::base::StringPrintf("--split-info=%s", split_info_file.path);
std::string debug_dir_arg = android::base::StringPrintf("--debug-dir=%s", debug_dir.path);
std::vector<const char*> args = {
"imgdiff", "-z", "--block-limit=10", split_info_arg.c_str(), debug_dir_arg.c_str(),
src_file.path, tgt_file.path, patch_file.path,
};
ASSERT_EQ(0, imgdiff(args.size(), args.data()));
std::string tgt;
ASSERT_TRUE(android::base::ReadFileToString(tgt_file.path, &tgt));
// Expect two split src images:
// src_piece 0: a-0 10 blocks
// src_piece 1: a-0 1 block, CD
GenerateAndCheckSplitTarget(debug_dir.path, 2, tgt);
}