Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More C++ tweaks and changes #9478

Merged
merged 12 commits into from
Dec 5, 2024
Merged

More C++ tweaks and changes #9478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6e47303
Playing around with some ideas
grendello Oct 31, 2024
5cfaf38
More consistent use of std::string_view + remove unused code
grendello Nov 4, 2024
96b6e42
Making EmbeddedAssemblies a static type
grendello Nov 7, 2024
80154b5
More cleanup
grendello Nov 13, 2024
4218e1b
More move to static, more cleanup
grendello Nov 13, 2024
ab57170
MonodroidRuntime is static now
grendello Nov 13, 2024
62663e9
Buglets fixed
grendello Nov 14, 2024
e20cbdd
Trying to find out why AppContext.GetData doesn't appear to work
grendello Nov 21, 2024
b4d4572
Cleanup
grendello Nov 21, 2024
41bb650
Lengths and offsets...
grendello Nov 25, 2024
9794be2
Cleanup
grendello Nov 26, 2024
d9d80ea
Address feedback
grendello Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/native/monodroid/debug.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ Debug::start_debugging (void)
if (sdb_fd == 0)
return;

embeddedAssemblies.set_register_debug_symbols (true);
EmbeddedAssemblies::set_register_debug_symbols (true);

char *debug_arg = Util::monodroid_strdup_printf ("--debugger-agent=transport=socket-fd,address=%d,embedding=1", sdb_fd);
std::array<char*, 2> debug_options = {
Expand Down Expand Up @@ -603,7 +603,7 @@ Debug::enable_soft_breakpoints (void)
void*
xamarin::android::conn_thread (void *arg)
{
abort_if_invalid_pointer_argument (arg);
abort_if_invalid_pointer_argument (arg, "arg");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how meaningful this will be "without context": the abort messages which may be logged to the Google Play Store (594d090) will only contain the message Parameter 'arg' must be a valid pointer. Which "arg"?

I think the parameter should also include the method name in the message. so that a "context free" (no adb logcat) message at least lets us know what function we're talking about:

Parameter 'arg' in function 'conn_thread' must be a valid pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in the latest commit


int res;
Debug *instance = static_cast<Debug*> (arg);
Expand Down
22 changes: 11 additions & 11 deletions src/native/monodroid/embedded-assemblies-zip.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ EmbeddedAssemblies::zip_load_assembly_store_entries (std::vector<uint8_t> const&
dynamic_local_string<SENSIBLE_PATH_MAX> entry_name;
bool assembly_store_found = false;

log_debug (LOG_ASSEMBLY, "Looking for assembly stores in APK ('%s)", assembly_store_file_path.data ());
log_debug (LOG_ASSEMBLY, "Looking for assembly stores in APK ('%s')", assembly_store_file_path.data ());
for (size_t i = 0uz; i < num_entries; i++) {
if (all_required_zip_entries_found ()) {
need_to_scan_more_apks = false;
Expand Down Expand Up @@ -298,7 +298,7 @@ EmbeddedAssemblies::zip_load_assembly_store_entries (std::vector<uint8_t> const&
}

void
EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, [[maybe_unused]] monodroid_should_register should_register)
EmbeddedAssemblies::zip_load_entries (int fd, const char *apk_name, [[maybe_unused]] monodroid_should_register should_register) noexcept
{
uint32_t cd_offset;
uint32_t cd_size;
Expand Down Expand Up @@ -412,7 +412,7 @@ EmbeddedAssemblies::set_debug_entry_data (XamarinAndroidBundledAssembly &entry,
}

bool
EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries)
EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries) noexcept
{
// The simplest case - no file comment
off_t ret = ::lseek (fd, -ZIP_EOCD_LEN, SEEK_END);
Expand Down Expand Up @@ -478,7 +478,7 @@ EmbeddedAssemblies::zip_read_cd_info (int fd, uint32_t& cd_offset, uint32_t& cd_
}

bool
EmbeddedAssemblies::zip_adjust_data_offset (int fd, ZipEntryLoadState &state)
EmbeddedAssemblies::zip_adjust_data_offset (int fd, ZipEntryLoadState &state) noexcept
{
static constexpr size_t LH_FILE_NAME_LENGTH_OFFSET = 26uz;
static constexpr size_t LH_EXTRA_LENGTH_OFFSET = 28uz;
Expand Down Expand Up @@ -530,7 +530,7 @@ EmbeddedAssemblies::zip_adjust_data_offset (int fd, ZipEntryLoadState &state)

template<size_t BufSize>
bool
EmbeddedAssemblies::zip_extract_cd_info (std::array<uint8_t, BufSize> const& buf, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries)
EmbeddedAssemblies::zip_extract_cd_info (std::array<uint8_t, BufSize> const& buf, uint32_t& cd_offset, uint32_t& cd_size, uint16_t& cd_entries) noexcept
{
constexpr size_t EOCD_TOTAL_ENTRIES_OFFSET = 10uz;
constexpr size_t EOCD_CD_SIZE_OFFSET = 12uz;
Expand Down Expand Up @@ -558,7 +558,7 @@ EmbeddedAssemblies::zip_extract_cd_info (std::array<uint8_t, BufSize> const& buf

template<class T>
force_inline bool
EmbeddedAssemblies::zip_ensure_valid_params (T const& buf, size_t index, size_t to_read) const noexcept
EmbeddedAssemblies::zip_ensure_valid_params (T const& buf, size_t index, size_t to_read) noexcept
{
if (index + to_read > buf.size ()) {
log_error (LOG_ASSEMBLY, "Buffer too short to read %u bytes of data", to_read);
Expand All @@ -570,7 +570,7 @@ EmbeddedAssemblies::zip_ensure_valid_params (T const& buf, size_t index, size_t

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint16_t& dst) const noexcept
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint16_t& dst) noexcept
{
if (!zip_ensure_valid_params (src, source_index, sizeof (dst))) {
return false;
Expand All @@ -583,7 +583,7 @@ EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint16_t&

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint32_t& dst) const noexcept
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint32_t& dst) noexcept
{
if (!zip_ensure_valid_params (src, source_index, sizeof (dst))) {
return false;
Expand All @@ -600,7 +600,7 @@ EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, uint32_t&

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, std::array<uint8_t, 4>& dst_sig) const noexcept
EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, std::array<uint8_t, 4>& dst_sig) noexcept
{
if (!zip_ensure_valid_params (src, source_index, dst_sig.size ())) {
return false;
Expand All @@ -612,7 +612,7 @@ EmbeddedAssemblies::zip_read_field (T const& src, size_t source_index, std::arra

template<ByteArrayContainer T>
bool
EmbeddedAssemblies::zip_read_field (T const& buf, size_t index, size_t count, dynamic_local_string<SENSIBLE_PATH_MAX>& characters) const noexcept
EmbeddedAssemblies::zip_read_field (T const& buf, size_t index, size_t count, dynamic_local_string<SENSIBLE_PATH_MAX>& characters) noexcept
{
if (!zip_ensure_valid_params (buf, index, count)) {
return false;
Expand All @@ -623,7 +623,7 @@ EmbeddedAssemblies::zip_read_field (T const& buf, size_t index, size_t count, dy
}

bool
EmbeddedAssemblies::zip_read_entry_info (std::vector<uint8_t> const& buf, dynamic_local_string<SENSIBLE_PATH_MAX>& file_name, ZipEntryLoadState &state)
EmbeddedAssemblies::zip_read_entry_info (std::vector<uint8_t> const& buf, dynamic_local_string<SENSIBLE_PATH_MAX>& file_name, ZipEntryLoadState &state) noexcept
{
constexpr size_t CD_COMPRESSION_METHOD_OFFSET = 10uz;
constexpr size_t CD_UNCOMPRESSED_SIZE_OFFSET = 24uz;
Expand Down
24 changes: 11 additions & 13 deletions src/native/monodroid/embedded-assemblies.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class MonoGuidString final
char *guid = nullptr;
};

void EmbeddedAssemblies::set_assemblies_prefix (const char *prefix)
void EmbeddedAssemblies::set_assemblies_prefix (const char *prefix) noexcept
{
if (assemblies_prefix_override != nullptr)
delete[] assemblies_prefix_override;
Expand Down Expand Up @@ -493,25 +493,25 @@ MonoAssembly*
EmbeddedAssemblies::open_from_bundles (MonoAssemblyLoadContextGCHandle alc_gchandle, MonoAssemblyName *aname, [[maybe_unused]] char **assemblies_path, [[maybe_unused]] void *user_data, MonoError *error)
{
constexpr bool ref_only = false;
return embeddedAssemblies.open_from_bundles (aname, alc_gchandle, error, ref_only);
return EmbeddedAssemblies::open_from_bundles (aname, alc_gchandle, error, ref_only);
}

MonoAssembly*
EmbeddedAssemblies::open_from_bundles_full (MonoAssemblyName *aname, [[maybe_unused]] char **assemblies_path, [[maybe_unused]] void *user_data)
{
constexpr bool ref_only = false;

return embeddedAssemblies.open_from_bundles (aname, ref_only /* loader_data */, nullptr /* error */, ref_only);
return EmbeddedAssemblies::open_from_bundles (aname, ref_only /* loader_data */, nullptr /* error */, ref_only);
}

void
EmbeddedAssemblies::install_preload_hooks_for_appdomains ()
EmbeddedAssemblies::install_preload_hooks_for_appdomains () noexcept
{
mono_install_assembly_preload_hook (open_from_bundles_full, nullptr);
}

void
EmbeddedAssemblies::install_preload_hooks_for_alc ()
EmbeddedAssemblies::install_preload_hooks_for_alc () noexcept
{
mono_install_assembly_preload_hook_v3 (
open_from_bundles,
Expand Down Expand Up @@ -679,7 +679,7 @@ EmbeddedAssemblies::typemap_java_to_managed (hash_t hash, const MonoString *java
} else {
MonoAssemblyLoadContextGCHandle alc_gchandle = mono_alc_get_default_gchandle ();
MonoError mono_error;
assm = embeddedAssemblies.open_from_bundles (assembly_name, alc_gchandle, &mono_error, false /* ref_only */);
assm = EmbeddedAssemblies::open_from_bundles (assembly_name, alc_gchandle, &mono_error, false /* ref_only */);
}

if (assm == nullptr) {
Expand Down Expand Up @@ -912,7 +912,7 @@ EmbeddedAssemblies::md_mmap_apk_file (int fd, uint32_t offset, size_t size, cons
}

void
EmbeddedAssemblies::gather_bundled_assemblies_from_apk (const char* apk, monodroid_should_register should_register)
EmbeddedAssemblies::gather_bundled_assemblies_from_apk (const char* apk, monodroid_should_register should_register) noexcept
{
int fd;

Expand Down Expand Up @@ -1284,12 +1284,10 @@ EmbeddedAssemblies::register_from_filesystem (const char *lib_dir_path,bool look
}

auto register_fn =
application_config.have_assembly_store ? std::mem_fn (&EmbeddedAssemblies::maybe_register_blob_from_filesystem) :
application_config.have_assembly_store ? &EmbeddedAssemblies::maybe_register_blob_from_filesystem :
(look_for_mangled_names ?
std::mem_fn (&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<true>) :
std::mem_fn (&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<false>
)
);
&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<true> :
&EmbeddedAssemblies::maybe_register_assembly_from_filesystem<false>);

size_t assembly_count = 0uz;
do {
Expand Down Expand Up @@ -1334,7 +1332,7 @@ EmbeddedAssemblies::register_from_filesystem (const char *lib_dir_path,bool look
}

// We get `true` if it's time to terminate
if (register_fn (this, should_register, assembly_count, cur, state)) {
if (register_fn (should_register, assembly_count, cur, state)) {
break;
}
} while (true);
Expand Down
Loading
Loading