Commit graph

30 commits

Author SHA1 Message Date
Tao Bao
7ebef8fd40 applypatch: Fix comparison of integers of different signs.
bootable/recovery/applypatch/imgpatch.cpp:57:3: error: comparison of integers of different signs: 'unsigned int' and 'int' [-Werror,-Wsign-compare]
  CHECK_GT(expected_target_length, 0);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

bootable/recovery/applypatch/freecache.cpp:145:50: error: comparison of integers of different signs: 'long' and '__fsblkcnt64_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
  if (sf.f_bsize == 0 || free_space / sf.f_bsize != sf.f_bavail) {
                         ~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~
bootable/recovery/applypatch/freecache.cpp:190:16: error: comparison of integers of different signs: 'int64_t' (aka 'long') and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
  if (free_now >= bytes_needed) {
      ~~~~~~~~ ^  ~~~~~~~~~~~~
bootable/recovery/applypatch/freecache.cpp:233:18: error: comparison of integers of different signs: 'int64_t' (aka 'long') and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]
    if (free_now >= bytes_needed) {
        ~~~~~~~~ ^  ~~~~~~~~~~~~

Test: `mmma -j bootable/recovery/applypatch` with -Wsign-compare
Test: Run recovery_unit_test on marlin.
Change-Id: I4aa1fd0f9b7205b9e4e50874fc4bccb62951e7fe
2018-12-17 10:07:27 -08:00
Tianjie Xu
7326892c7d Remove the debug code for bspatch flakiness
We already know the flakiness happens in bspatch, and the issue is
tracked in b/80193170.

Bug: 67849209
Test: unit tests pass
Change-Id: Ib4772b8f2f0225125096fe7407d083b5bb542cfb
2018-07-11 11:51:43 -07:00
Tao Bao
511d759627 edify: Remove VAL_INVALID and move ValueType into Value class.
Test: mmma -j bootable/recovery
Test: Run recovery_component_test and recovery_unit_test on marlin.
Change-Id: I4b240e3e771c387b9694be9c0f2f74e0265ab4cb
2018-07-09 23:20:30 -07:00
Tianjie Xu
cc61cf6a9f Convert deflate image chunks to raw if the raw data is smaller
The imgpatch will fail on empty deflates because the bspatch won't call
the sink function if the target length is zero.

Instead of compressing an empty string, it's cleaner to not generate such
empty deflate chunks in the patch. Therefore, we can just convert the
chunk type to raw if the target length is smaller than the patch data.

Also adjust some unit tests and add the testdata gzipped_source &
gzipped_target. These two files are ~1K each and are generated by
gzipping two slightly different regular files.

Bug: 79265132
Test: unit tests pass, imgpatch applys successfully on the given src/tgt
Change-Id: I6bfff3251918137f6762a6f9e9551642371a1124
2018-05-24 10:49:54 -07:00
Tianjie Xu
3f638ee834 Save the target file when applypatch tests fail
Save the target file to tempfile upon unittest failures so that we can
try to decompress the deflate chunks in the flaky unittests. And print
the zlib version in case that gets changed.

Also the SHA1 of the uncompressed data seems correct; so only keep the
final SHA1 to double confirm.

Bug: 67849209
Test: recovery_component_test
Change-Id: Ic6447c2b75c29379d6844cd23a0ff1c4305694a0
2018-04-26 19:46:39 -07:00
Tianjie Xu
9e1ccd47b4 Dump the uncompressed data's SHA1 to debug flaky tests
Dump the SHA1 of the uncompressed data in applypatch to confirm if we
are at least doing the bspatch part correctly. (I expect so since the actual
length of the uncompressed data matches the expected length).

Also try to decompress the deflate chunk inside the recovery image for
these two flacky tests. In theory, there shouldn't be randomness in
zlib; so we would know if we process the data wrongly if the deflate fails
to decompress.

Bug: 67849209
Test: recovery_component_test
Change-Id: Id947522153b1eeb0d10d161298a96fb045f92018
2018-04-25 20:00:56 -07:00
Tianjie Xu
ffed57a7a3 Dump debug information for apply_patch unit tests
The apply patch test should have a deterministic way to append patch
data. Add debug logs to dump the length and SHA1 of each step to further
track down the flakiness.

Also redirect the debug logging to stdout in case the logcat becomes too
chatty.

Bug: 67849209
Test: Run recovery_component_test
Change-Id: I42bafef2d9dee599719ae57840b3d8c00d243ebd
2018-04-24 09:56:55 -07:00
Tao Bao
8b0b0f1f02 applypatch: Drop the SHA_CTX parameter in Apply{BSDiff,Image}Patch.
As they're accepting the SinkFn callback, it makes more sense to leave
the work to their callers.

Test: mmma -j bootable/recovery
Test: Run recovery_component_test on marlin.
Test: No other active user of the two functions.
Change-Id: I8d67b38ce037925442296f136b483e0c71983777
2018-04-20 09:27:50 -07:00
Tao Bao
1e0941f4f6 applypatch: Change the patch parameter to const Value& in Apply{BSDiff,Image}Patch.
It used to be "const Value*", but nullptr won't be a valid input.

Test: recovery_host_test; recovery_component_test
Change-Id: I904b5689ac3e64504088bf0544c9fb5d45a52243
2017-11-10 12:18:34 -08:00
Tao Bao
fdec103335 applypatch: Fix a memory leak in ApplyImagePatch().
$ valgrind --leak-check=full out/host/linux-x86/nativetest64/recovery_host_test/recovery_host_test

==36755== 112 bytes in 1 blocks are definitely lost in loss record 4 of 16
==36755==    at 0x40307C4: malloc (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:270)
==36755==    by 0x40C1669: operator new(unsigned long) (external/libcxxabi/src/cxa_new_delete.cpp:46)
==36755==    by 0x18D6A8: ApplyImagePatch(unsigned char const*, unsigned long, Value const*, std::__1::function<unsigned long (unsigned char const*, unsigned long)>, sha_state_st*, Value const*) (bootable/recovery/applypatch/imgpatch.cpp:62)
==36755==    by 0x18D02B: ApplyImagePatch(unsigned char const*, unsigned long, unsigned char const*, unsigned long, std::__1::function<unsigned long (unsigned char const*, unsigned long)>) (bootable/recovery/applypatch/imgpatch.cpp:134)
==36755==    by 0x160D15: GenerateTarget(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*) (bootable/recovery/tests/component/imgdiff_test.cpp:85)
==36755==    by 0x11FA7D: verify_patched_image(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) (bootable/recovery/tests/component/imgdiff_test.cpp:96)
==36755==    by 0x12966C: ImgdiffTest_zip_mode_smoke_trailer_zeros_Test::TestBody() (bootable/recovery/tests/component/imgdiff_test.cpp:295)
==36755==    by 0x235EF9: testing::Test::Run() (external/googletest/googletest/src/gtest.cc:2455)
==36755==    by 0x236CBF: testing::TestInfo::Run() (external/googletest/googletest/src/gtest.cc:2653)
==36755==    by 0x2372D6: testing::TestCase::Run() (external/googletest/googletest/src/gtest.cc:2771)
==36755==    by 0x23EEE6: testing::internal::UnitTestImpl::RunAllTests() (external/googletest/googletest/src/gtest.cc:4648)
==36755==    by 0x23EB45: testing::UnitTest::Run() (external/googletest/googletest/src/gtest.cc:2455)

std::unique_ptr<z_stream, decltype(&deflateEnd)> strm(new z_stream(), deflateEnd);

Only the internally allocated buffers inside 'strm' would be free'd by
deflateEnd(), but not 'strm' itself.

This CL fixes the issue by moving 'strm' to stack variable. Note that we
only need to call deflateEnd() on successful return of deflateInit2().

Test: recovery_host_test && recovery_component_test
Change-Id: I39b9bdf62376b8029f95cab82c8542bfcb874009
2017-10-24 12:48:36 -07:00
Tao Bao
38d78d19b9 applypatch: Forward declare struct Value.
And move '#include "edify/expr.h"' into .cpp files. This breaks the
transitive dependency on libedify. Modules that include
"applypatch/applypatch.h" don't need to add libedify into their
dependency list, unless they really need anything from libedify.

Build libedify static library for host, which is needed by
libimgpatch.

Test: mmma bootable/recovery
Change-Id: Ibb53d322579fcbf593438d058d9bcee240625941
2017-10-09 11:50:09 -07:00
Tianjie Xu
a897f95bd5 Implement a custom deflate sink function for bspatch
This new sink function works as a wrapper of the old sink. It deflates
the available patch data on the fly. Therefore, we don't need to store
the full uncompressed patch data in memory.

Test: recovery_component_test && apply an incremental update on angler
Change-Id: I2274ec50a1607089abcc9d0954a2a748f28c3122
2017-05-26 14:05:20 -07:00
Tianjie Xu
ce5fa5e538 Print SHA1 of the patch if bsdiff fails with data error
This will help us to identify the patch corruption.

Meanwhile fix a wrong size parameter passed to bspatch.
(patch->data.size() into patch->data.size() - patch_offset).

Also remove the only usage of "ApplyBSDiffPatchMem()" and inline its
Sink function for simplicity.

Bug: 37855643
Test: Prints SHA1 for corrupted patch in imgdiff_test.
Change-Id: Ibf2db8c08b0ded1409bb7c91a3547a6bf99c601d
2017-05-16 12:39:14 -07:00
Jinguang Dong
391bb7bb92 applypatch: Add determine the return value of ApplyDiffPatch and
capture the error flow.

Construct ota package which is bsdiff exception scene ,then do
simulation test, native code can not capture exception scenes.

Test: recovery_component_test
Test: Apply an bsdiff exception scene ota package.
Change-Id: Icd9f6eac78739bd35c74b9fcaaf8154335d680a5
2017-04-26 10:59:57 +08:00
Tao Bao
c0e1c46a70 applypatch: Let Apply{BSDiff,Image}Patch accept std::function.
Test: mmma bootable/recovery system/update_engine
Test: recovery_component_test
Change-Id: I93c2caa87bf94a53509bb37f98f2c02bcadb6f5c
2017-03-28 10:14:53 -07:00
Tao Bao
f7eb760fe7 applypatch: Change the ssize_t length parameters to size_t.
Mostly for applypatch family APIs like ApplyBSDiffPatch() and
ApplyImagePatch(). Changing to size_t doesn't indicate they would
necessarily work with very large size_t (e.g. > ssize_t), just
similar to write(2). But otherwise accepting negative length doesn't
make much sense.

Also change the return type of SinkFn from ssize_t to size_t. Callers
tell a successful sink by comparing the number of written bytes against
the desired value. Negative return values like -1 are not needed. This
also makes it consistent with bsdiff::bspatch interface.

Test: recovery_component_test
Test: Apply an incremental with the new updater.
Change-Id: I7ff1615203a5c9854134f75d019e266f4ea6e714
2017-03-28 10:13:38 -07:00
Tianjie Xu
12b90553d7 More cleanup to imgdiff & imgpatch
Also remove the utils in applypatch and replace them with the
corresponding libbase functions.

Test: recovery tests pass.
Change-Id: I77254c141bd3e7d3d6894c23b60e866009516f81
2017-03-16 12:09:49 -07:00
Sen Jiang
25c56979dd Use bspatch from external/bsdiff.
Now ApplyBSDiffPatch() will stream the output to sink as we go instead
of sinking everything at the end.

Test: recovery_host_test
Bug: 26982501

Change-Id: I05b6ed40d45e4b1b19ae72784cf705b731b976e3
2017-02-02 14:41:05 -08:00
Tao Bao
087bc0c7d3 imgpatch: Compile with ZLIB_CONST defined.
So z_stream.next_in takes pointer to const data.

Test: mmma bootable/recovery/applypatch
Change-Id: If269b766a7c84fa2f67424ee61ba5afab0159261
2017-01-20 12:13:28 -08:00
Tao Bao
97555da4a6 Add tests for imgdiff.
Factor out libimgdiff static library for testing purpose.

This CL adds the imgdiff tests on host and on target both (similar to
libimgpatch). In practice, we only need imgdiff binary on host, and
libimgpatch on target. But they should build and pass tests on both
platforms.

Test: recovery_host_test passes; recovery_component_test passes.
Change-Id: I0eafb7faf727cdf70066310e845af6ee245d4f60
2016-12-19 16:53:03 -08:00
Tianjie Xu
aced5d9e4e Change StringValue to use std::string
Changing the field of 'Value' in edify to std::string from char*.
Meanwhile cleaning up the users of 'Value' and switching them to
cpp style.

Test: compontent tests passed.
Bug: 31713288

Change-Id: Iec5a7d601b1e4ca40935bf1c70d325dafecec235
2016-10-15 01:18:23 +00:00
Tao Bao
38afad46d8 resolve merge conflicts of 490fad6 to nyc-dev-plus-aosp
Change-Id: I299fe15977c1a59d0c784728872c3a7f63c95e56
2016-06-13 19:12:46 -07:00
Tao Bao
490fad6791 applypatch: Don't call inflate() when it expects zero-length output.
We may have expanded_len == 0 when calling inflate(). After switching to
using std::vector, it passes a nullptr buffer to inflate() and leads to
Z_STREAM_ERROR.

Bug: 29312140
Change-Id: Iab7c6c07a9e8488e844e7cdda76d02bd60d2ea98
2016-06-13 16:42:52 -07:00
Chih-Hung Hsieh
54a2747ef3 Fix google-runtime-int warnings.
Bug: 28220065
Change-Id: Ida199c66692a1638be6990d583d2ed42583fb592
2016-04-18 12:29:30 -07:00
Tao Bao
d80a99883d Fix the improper use of LOCAL_WHOLE_STATIC_LIBRARIES.
If two libraries both use LOCAL_WHOLE_STATIC_LIBRARIES and include a same
library, there would be linking errors when generating a shared library
(or executable) that depends on the two libraries both.

Also clean up Android.mk files.

Remove the "LOCAL_MODULE_TAGS := eng" line for the updater module. The
module will then default to "optional" which won't be built until needed.

Change-Id: I3ec227109b8aa744b7568e7f82f575aae3fe0e6f
2016-03-03 14:52:44 -08:00
Sen Jiang
696692a3c9 applypatch: Add a Makefile to build imgdiff in Chrome OS.
Also fixed some warnings and added check for target_len.

Test: mma; emerge-peppy imgdiff; emerge-nyan imgdiff; sudo emerge imgdiff
Bug: 26866274

Change-Id: Ifbcd3afd6701c769ccf626e33ed94461706f7ee6
2016-02-09 15:26:57 -08:00
Yabin Cui
d483c20a7e applypatch: fix memory leaks reported by static analysis.
Bug: 26906416
Change-Id: I163df5a8f3abda3ba5d4ed81dfc8567054eceb27
2016-02-04 15:42:02 -08:00
Sen Jiang
c48cb5e597 Switch from mincrypt to BoringSSL in applypatch and updater.
Bug: 18790686
Change-Id: I7d2136fb39b2266f5ae5be24819c617b08a6c21e
2016-02-04 16:27:43 +08:00
Sen Jiang
0cce9cda0c applypatch: Compile libimgpatch for target and host.
update_engine need it for the new IMGDIFF operation.

Also removed __unused in ApplyImagePatch() as I got error building it
for the host, and I think it's dangerous not checking the size of the
input.

Test: mma
Bug: 26628339

Change-Id: I22d4cd55c2c3f87697d6afdf10e8106fef7d1a9c
2016-01-28 13:26:25 +08:00
Tao Bao
ba9a42aa7e recovery: Switch applypatch/ and updater/ to cpp.
Mostly trivial changes to make cpp compiler happy.

Change-Id: I69bd1d96fcccf506007f6144faf37e11cfba1270
2015-07-13 17:21:31 -07:00
Renamed from applypatch/imgpatch.c (Browse further)