Skip to content

Commit

Permalink
libmbcommon: Use bitmask to enable multiple formats and drop set_form…
Browse files Browse the repository at this point in the history 
…at()

set_format() never worked properly due to it causing the bidding process
to be skipped and every FormatReader implementation required open() to
be called.

Signed-off-by: Andrew Gunnerson <[email protected]>
  • Loading branch information
@chenxiaolong
chenxiaolong committed Nov 28, 2018
1 parent d2b9659 commit 86bd65d
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 137 deletions.
4 changes: 2 additions & 2 deletions bootimgtool/bootimgtool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -829,13 +829,13 @@ static bool unpack_main(int argc, char *argv[])
return false;
}

if (auto r = reader.enable_format(*format); !r) {
if (auto r = reader.enable_formats(*format); !r) {
fprintf(stderr, "Failed to enable format '%s': %s\n",
type, r.error().message().c_str());
return false;
}
} else {
if (auto r = reader.enable_format_all(); !r) {
if (auto r = reader.enable_formats_all(); !r) {
fprintf(stderr, "Failed to enable all formats: %s\n",
r.error().message().c_str());
return false;
Expand Down
4 changes: 2 additions & 2 deletions examples/bootimg_compare.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,12 @@ int main(int argc, char *argv[])
size_t entries = 0;

// Set up reader formats
if (auto r = reader1.enable_format_all(); !r) {
if (auto r = reader1.enable_formats_all(); !r) {
fprintf(stderr, "Failed to enable all boot image formats: %s\n",
r.error().message().c_str());
return EXIT_FAILURE;
}
if (auto r = reader2.enable_format_all(); !r) {
if (auto r = reader2.enable_formats_all(); !r) {
fprintf(stderr, "Failed to enable all boot image formats: %s\n",
r.error().message().c_str());
return EXIT_FAILURE;
Expand Down
25 changes: 16 additions & 9 deletions libmbbootimg/include/mbbootimg/format.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,31 @@
#include <string_view>

#include "mbcommon/common.h"
#include "mbcommon/flags.h"

namespace mb::bootimg
{

enum class Format
enum class Format : uint8_t
{
Android,
Bump,
Loki,
Mtk,
SonyElf,
Android = 1 << 0,
Bump = 1 << 1,
Loki = 1 << 2,
Mtk = 1 << 3,
SonyElf = 1 << 4,
};
MB_DECLARE_FLAGS(Formats, Format)
MB_DECLARE_OPERATORS_FOR_FLAGS(Formats)

constexpr Formats ALL_FORMATS =
Format::Android
| Format::Bump
| Format::Loki
| Format::Mtk
| Format::SonyElf;

MB_EXPORT std::string_view format_to_name(Format format);

MB_EXPORT std::optional<Format> name_to_format(std::string_view name);

// TODO: Switch to span eventually
MB_EXPORT std::basic_string_view<Format> formats();

}
6 changes: 2 additions & 4 deletions libmbbootimg/include/mbbootimg/reader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ class MB_EXPORT Reader

// Format operations
std::optional<Format> format();
oc::result<void> enable_format(Format format);
oc::result<void> enable_format_all();
oc::result<void> set_format(Format format);
oc::result<void> enable_formats(Formats formats);
oc::result<void> enable_formats_all();

// Reader state
bool is_open();
Expand All @@ -85,7 +84,6 @@ class MB_EXPORT Reader

std::vector<std::unique_ptr<detail::FormatReader>> m_formats;
detail::FormatReader *m_format;
bool m_format_user_set;
};

}
Expand Down
5 changes: 2 additions & 3 deletions libmbbootimg/include/mbbootimg/reader_error.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ enum class ReaderError
UnknownOption = 20,

// Format errors
FormatAlreadyEnabled = 30,
NoFormatsRegistered = 31,
UnknownFileFormat = 32,
NoFormatsRegistered = 30,
UnknownFileFormat = 31,

EndOfEntries = 40,

Expand Down
31 changes: 14 additions & 17 deletions libmbbootimg/src/format.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,20 @@ namespace mb::bootimg
* \brief Format code for Sony ELF boot images
*/

static std::array<Format, 5> g_formats{{
Format::Android,
Format::Bump,
Format::Loki,
Format::Mtk,
Format::SonyElf,
}};
/*!
* \class Formats
*
* \brief Bitmask of Format variants
*/

/*!
* \var ALL_FORMATS
*
* \brief Bitmask of all supported formats
*
* Note that this Formats object can be iterated over to get each supported
* \ref Format variant.
*/

/*!
* \brief Convert format to string
Expand Down Expand Up @@ -130,14 +137,4 @@ std::optional<Format> name_to_format(std::string_view name)
}
}

/*!
* \brief Get list of supported formats
*
* \return View to static list of supported formats
*/
std::basic_string_view<Format> formats()
{
return {g_formats.data(), g_formats.size()};
}

}
147 changes: 55 additions & 92 deletions libmbbootimg/src/reader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ Reader::Reader()
, m_owned_file()
, m_file()
, m_format()
, m_format_user_set(false)
{
}

Expand All @@ -240,24 +239,21 @@ Reader::Reader(Reader &&other) noexcept
std::swap(m_file, other.m_file);
std::swap(m_formats, other.m_formats);
std::swap(m_format, other.m_format);
std::swap(m_format_user_set, other.m_format_user_set);
}

Reader & Reader::operator=(Reader &&rhs) noexcept
{
if (this != &rhs) {
(void) close();

// close() only resets the autodetected format
// close() doesn't reset enabled formats
m_formats.clear();
m_format = nullptr;

std::swap(m_state, rhs.m_state);
std::swap(m_owned_file, rhs.m_owned_file);
std::swap(m_file, rhs.m_file);
std::swap(m_formats, rhs.m_formats);
std::swap(m_format, rhs.m_format);
std::swap(m_format_user_set, rhs.m_format_user_set);
}

return *this;
Expand Down Expand Up @@ -349,42 +345,40 @@ oc::result<void> Reader::open(File *file)
}
});

// Perform bid if a format wasn't explicitly chosen
if (!m_format) {
for (auto &f : m_formats) {
// Seek to beginning
OUTCOME_TRYV(file->seek(0, SEEK_SET));
// Perform bid for autodetection
for (auto &f : m_formats) {
// Seek to beginning
OUTCOME_TRYV(file->seek(0, SEEK_SET));

auto close_f = finally([&] {
(void) f->close(*file);
});
auto close_f = finally([&] {
(void) f->close(*file);
});

// Call bidder
OUTCOME_TRY(bid, f->open(*file, best_bid));
// Call bidder
OUTCOME_TRY(bid, f->open(*file, best_bid));

if (bid > best_bid) {
// Close previous best format
if (format) {
(void) format->close(*file);
}
if (bid > best_bid) {
// Close previous best format
if (format) {
(void) format->close(*file);
}

// Don't close this format
close_f.dismiss();
// Don't close this format
close_f.dismiss();

best_bid = bid;
format = f.get();
}
best_bid = bid;
format = f.get();
}
}

if (!format) {
return ReaderError::UnknownFileFormat;
}
if (!format) {
return ReaderError::UnknownFileFormat;
}

// We've found a matching format, so don't close it
close_format.dismiss();
// We've found a matching format, so don't close it
close_format.dismiss();

m_format = format;
}
m_format = format;

m_state = ReaderState::Header;
m_file = file;
Expand All @@ -409,10 +403,7 @@ oc::result<void> Reader::close()
m_owned_file.reset();
m_file = nullptr;

// Reset auto-detected format
if (!m_format_user_set) {
m_format = nullptr;
}
m_format = nullptr;
});

oc::result<void> ret = oc::success();
Expand Down Expand Up @@ -509,12 +500,7 @@ oc::result<size_t> Reader::read_data(void *buf, size_t size)
}

/*!
* \brief Get detected or forced boot image format code.
*
* * If enable_format() was used, then the detected boot image format code is
* returned.
* * If set_format() was used, then the forced boot image format code is
* returned.
* \brief Get detected boot image format code.
*
* \note The return value is meaningful only after the boot image has been
* successfully opened.
Expand Down Expand Up @@ -549,77 +535,54 @@ static std::unique_ptr<FormatReader> _construct_format(Format format)
}

/*!
* \brief Enable support for a boot image format.
* \brief Enable support for boot image formats.
*
* \note Calling this function replaces previously enabled formats. It is not
* cumulative.
*
* \param format Format to enable
* \param formats Formats to enable
*
* \return Nothing if the format is successfully enabled. Otherwise, the error
* \return Nothing if the formats are successfully enabled. Otherwise, the error
* code.
*/
oc::result<void> Reader::enable_format(Format format)
oc::result<void> Reader::enable_formats(Formats formats)
{
ENSURE_STATE_OR_RETURN_ERROR(ReaderState::New);

for (auto const &f : m_formats) {
if (f->type() == format) {
return ReaderError::FormatAlreadyEnabled;
if (formats != (formats & ALL_FORMATS)) {
return std::errc::invalid_argument;
}

// Remove formats that shouldn't be enabled
for (auto it = m_formats.begin(); it != m_formats.end();) {
auto format = (*it)->type();
if (formats & format) {
formats.set_flag(format, false);
++it;
} else {
it = m_formats.erase(it);
}
}

m_formats.push_back(_construct_format(format));
// Enable what's left to be enabled
for (auto const &format : formats) {
m_formats.push_back(_construct_format(format));
}

return oc::success();
}

/*!
* \brief Enable support for all boot image formats.
*
* This is equivalent to calling `enable_formats(ALL_FORMATS)`.
*
* \return Nothing if all formats are successfully enabled. Otherwise, the error
* code.
*/
oc::result<void> Reader::enable_format_all()
{
ENSURE_STATE_OR_RETURN_ERROR(ReaderState::New);

for (auto const &format : formats()) {
auto ret = enable_format(format);
if (!ret && ret.error() != ReaderError::FormatAlreadyEnabled) {
return ret.as_failure();
}
}

return oc::success();
}

/*!
* \brief Force parsing as a specific boot image format.
*
* Calling this function causes the bidding process to be skipped. The chosen
* format will be used regardless of which formats are enabled.
*
* \param format Boot image format
*
* \return Nothing if the format is successfully set. Otherwise, the error code.
*/
oc::result<void> Reader::set_format(Format format)
oc::result<void> Reader::enable_formats_all()
{
ENSURE_STATE_OR_RETURN_ERROR(ReaderState::New);

auto ret = enable_format(format);
if (!ret && ret.error() != ReaderError::FormatAlreadyEnabled) {
return ret.as_failure();
}

for (auto &f : m_formats) {
if (f->type() == format) {
m_format = f.get();
m_format_user_set = true;

return oc::success();
}
}

MB_UNREACHABLE("Enabled format not found");
return enable_formats(ALL_FORMATS);
}

/*!
Expand Down
2 changes: 0 additions & 2 deletions libmbbootimg/src/reader_error.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ std::string ReaderErrorCategory::message(int ev) const
return "invalid state";
case ReaderError::UnknownOption:
return "unknown option";
case ReaderError::FormatAlreadyEnabled:
return "format already enabled";
case ReaderError::NoFormatsRegistered:
return "no formats registered";
case ReaderError::UnknownFileFormat:
Expand Down
2 changes: 1 addition & 1 deletion libmbbootimg/tests/format/test_android_reader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ struct AndroidReaderGoToEntryTest : testing::Test
// Write secondboot
memcpy(_data.data() + 3 * ahdr.page_size, "secondboot", 10);

ASSERT_TRUE(_reader.enable_format(Format::Android));
ASSERT_TRUE(_reader.enable_formats(Format::Android));

(void) _file.open(_data.data(), _data.size());
ASSERT_TRUE(_file.is_open());
Expand Down
Loading

0 comments on commit 86bd65d

Please sign in to comment.