Skip to content

Commit f26df9e

Browse files
authored
[native] More std::string_view use for consistency and safety (#10164)
This commit replaces use of raw `char*` pointers with `std::string_view` for consistency and safety in DSO loading and file/path utilities. * Converted DSO loading functions and related declarations to use `std::string_view` * Updated utility functions in `util.hh` with new `file_exists_no_null_check` and a `std::string_view` overload * Added `is_path_rooted` helper and adjusted calls to use `std::string_view` Using `std::string_view` removes ambiguity in handling string literals as well as in dealing with `char` arrays and pointers. `std::string_view` can also make code (very slightly) faster in situations when string length is required, since it stores string size and removes the need to use `strlen` on plain C++ strings.
1 parent 510789b commit f26df9e

File tree

4 files changed

+53
-29
lines changed

4 files changed

+53
-29
lines changed

src/native/clr/include/runtime-base/android-system.hh

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,16 +104,16 @@ namespace xamarin::android {
104104
static void detect_embedded_dso_mode (jstring_array_wrapper& appDirs) noexcept;
105105
static void setup_environment () noexcept;
106106
static void setup_app_library_directories (jstring_array_wrapper& runtimeApks, jstring_array_wrapper& appDirs, bool have_split_apks) noexcept;
107-
static auto load_dso (const char *path, unsigned int dl_flags, bool skip_exists_check) noexcept -> void*;
108-
static auto load_dso_from_any_directories (const char *name, unsigned int dl_flags) noexcept -> void*;
107+
static auto load_dso (std::string_view const& path, unsigned int dl_flags, bool skip_exists_check) noexcept -> void*;
108+
static auto load_dso_from_any_directories (std::string_view const& name, unsigned int dl_flags) noexcept -> void*;
109109

110110
private:
111-
static auto get_full_dso_path (std::string const& base_dir, const char *dso_path, dynamic_local_string<SENSIBLE_PATH_MAX>& path) noexcept -> bool;
111+
static auto get_full_dso_path (std::string const& base_dir, std::string_view const& dso_path, dynamic_local_string<SENSIBLE_PATH_MAX>& path) noexcept -> bool;
112112

113113
template<class TContainer> // TODO: replace with a concept
114-
static auto load_dso_from_specified_dirs (TContainer directories, const char *dso_name, unsigned int dl_flags) noexcept -> void*;
115-
static auto load_dso_from_app_lib_dirs (const char *name, unsigned int dl_flags) noexcept -> void*;
116-
static auto load_dso_from_override_dirs (const char *name, unsigned int dl_flags) noexcept -> void*;
114+
static auto load_dso_from_specified_dirs (TContainer directories, std::string_view const& dso_name, unsigned int dl_flags) noexcept -> void*;
115+
static auto load_dso_from_app_lib_dirs (std::string_view const& name, unsigned int dl_flags) noexcept -> void*;
116+
static auto load_dso_from_override_dirs (std::string_view const& name, unsigned int dl_flags) noexcept -> void*;
117117
static auto lookup_system_property (std::string_view const &name, size_t &value_len) noexcept -> const char*;
118118
static auto monodroid__system_property_get (std::string_view const&, char *sp_value, size_t sp_value_len) noexcept -> int;
119119
static auto get_max_gref_count_from_system () noexcept -> long;

src/native/clr/include/runtime-base/monodroid-dl.hh

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#pragma once
22

33
#include <mutex>
4+
#include <string_view>
45

56
#include <dlfcn.h>
67
#include <android/dlext.h>
@@ -71,7 +72,7 @@ namespace xamarin::android
7172
return find_dso_cache_entry_common<CacheKind::DSO> (hash);
7273
}
7374

74-
static auto monodroid_dlopen_log_and_return (void *handle, const char *full_name) -> void*
75+
static auto monodroid_dlopen_log_and_return (void *handle, std::string_view const& full_name) -> void*
7576
{
7677
if (handle == nullptr) {
7778
const char *load_error = dlerror ();
@@ -89,7 +90,7 @@ namespace xamarin::android
8990
return handle;
9091
}
9192

92-
static auto monodroid_dlopen_ignore_component_or_load (const char *name, int flags) noexcept -> void*
93+
static auto monodroid_dlopen_ignore_component_or_load (std::string_view const& name, int flags) noexcept -> void*
9394
{
9495
unsigned int dl_flags = static_cast<unsigned int>(flags);
9596
void * handle = AndroidSystem::load_dso_from_any_directories (name, dl_flags);
@@ -103,14 +104,14 @@ namespace xamarin::android
103104

104105
public:
105106
[[gnu::flatten]]
106-
static auto monodroid_dlopen (const char *name, int flags, bool prefer_aot_cache) noexcept -> void*
107+
static auto monodroid_dlopen (std::string_view const& name, int flags, bool prefer_aot_cache) noexcept -> void*
107108
{
108-
if (name == nullptr) {
109+
if (name.empty ()) [[unlikely]] {
109110
log_warn (LOG_ASSEMBLY, "monodroid_dlopen got a null name. This is not supported in NET+"sv);
110111
return nullptr;
111112
}
112113

113-
hash_t name_hash = xxhash::hash (name, strlen (name));
114+
hash_t name_hash = xxhash::hash (name.data (), name.size ());
114115
log_debug (LOG_ASSEMBLY, "monodroid_dlopen: hash for name '{}' is {:x}", name, name_hash);
115116

116117
DSOCacheEntry *dso = nullptr;
@@ -186,16 +187,16 @@ namespace xamarin::android
186187
}
187188

188189
[[gnu::flatten]]
189-
static auto monodroid_dlsym (void *handle, const char *name) -> void*
190+
static auto monodroid_dlsym (void *handle, std::string_view const& name) -> void*
190191
{
191192
char *e = nullptr;
192-
void *s = microsoft::java_interop::java_interop_lib_symbol (handle, name, &e);
193+
void *s = microsoft::java_interop::java_interop_lib_symbol (handle, name.data (), &e);
193194

194195
if (s == nullptr) {
195196
log_error (
196197
LOG_ASSEMBLY,
197198
"Could not find symbol '{}': {}",
198-
optional_string (name),
199+
name,
199200
optional_string (e)
200201
);
201202
}

src/native/clr/include/runtime-base/util.hh

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,30 @@ namespace xamarin::android {
9090
return false;
9191
}
9292

93+
static auto file_exists_no_null_check (const char *file) noexcept -> bool
94+
{
95+
return exists_and_is_mode (file, S_IFREG);
96+
}
97+
9398
public:
9499
static auto dir_exists (std::string_view const& dir_path) noexcept -> bool
95100
{
96101
return exists_and_is_mode (dir_path, S_IFDIR);
97102
}
98103

104+
[[gnu::flatten]]
105+
static auto file_exists (std::string_view const& file) noexcept -> bool
106+
{
107+
return file_exists_no_null_check (file.data ());
108+
}
109+
99110
static auto file_exists (const char *file) noexcept -> bool
100111
{
101112
if (file == nullptr) {
102113
return false;
103114
}
104115

105-
return exists_and_is_mode (file, S_IFREG);
116+
return file_exists_no_null_check (file);
106117
}
107118

108119
template<size_t MaxStackSize>
@@ -112,7 +123,7 @@ namespace xamarin::android {
112123
return false;
113124
}
114125

115-
return file_exists (file.get ());
126+
return file_exists_no_null_check (file.get ());
116127
}
117128

118129
static auto file_exists (int dirfd, std::string_view const& file) noexcept -> bool
@@ -247,6 +258,15 @@ namespace xamarin::android {
247258
return path [0] == '/';
248259
}
249260

261+
static auto is_path_rooted (std::string_view const& path) noexcept -> bool
262+
{
263+
if (path.empty ()) {
264+
return false;
265+
}
266+
267+
return path[0] == '/';
268+
}
269+
250270
private:
251271
// TODO: needs some work to accept mixed params of different accepted types
252272
template<size_t MaxStackSpace, detail::PathBuffer<MaxStackSpace> TBuffer, detail::PathComponentString ...TPart>

src/native/clr/runtime-base/android-system.cc

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -410,25 +410,27 @@ AndroidSystem::get_max_gref_count_from_system () noexcept -> long
410410
return max;
411411
}
412412

413-
auto AndroidSystem::get_full_dso_path (std::string const& base_dir, const char *dso_path, dynamic_local_string<SENSIBLE_PATH_MAX>& path) noexcept -> bool
413+
auto AndroidSystem::get_full_dso_path (std::string const& base_dir, std::string_view const& dso_path, dynamic_local_string<SENSIBLE_PATH_MAX>& path) noexcept -> bool
414414
{
415-
if (dso_path == nullptr) {
415+
if (dso_path.empty ()) {
416416
return false;
417417
}
418418

419-
if (base_dir.empty () || Util::is_path_rooted (dso_path))
420-
return const_cast<char*>(dso_path); // Absolute path or no base path, can't do much with it
419+
if (base_dir.empty () || Util::is_path_rooted (dso_path)) {
420+
path.assign (dso_path);
421+
return true; // Absolute path or no base path, can't do much with it
422+
}
421423

422424
path.assign (base_dir)
423425
.append ("/"sv)
424-
.append_c (dso_path);
426+
.append (dso_path);
425427

426428
return true;
427429
}
428430

429-
auto AndroidSystem::load_dso (const char *path, unsigned int dl_flags, bool skip_exists_check) noexcept -> void*
431+
auto AndroidSystem::load_dso (std::string_view const& path, unsigned int dl_flags, bool skip_exists_check) noexcept -> void*
430432
{
431-
if (path == nullptr || *path == '\0') {
433+
if (path.empty ()) [[unlikely]] {
432434
return nullptr;
433435
}
434436

@@ -439,18 +441,19 @@ auto AndroidSystem::load_dso (const char *path, unsigned int dl_flags, bool skip
439441
}
440442

441443
char *error = nullptr;
442-
void *handle = java_interop_lib_load (path, dl_flags, &error);
444+
void *handle = java_interop_lib_load (path.data (), dl_flags, &error);
443445
if (handle == nullptr && Util::should_log (LOG_ASSEMBLY)) {
444446
log_info_nocheck_fmt (LOG_ASSEMBLY, "Failed to load shared library '{}'. {}", path, error);
445447
}
446448
java_interop_free (error);
449+
447450
return handle;
448451
}
449452

450453
template<class TContainer> [[gnu::always_inline]] // TODO: replace with a concept
451-
auto AndroidSystem::load_dso_from_specified_dirs (TContainer directories, const char *dso_name, unsigned int dl_flags) noexcept -> void*
454+
auto AndroidSystem::load_dso_from_specified_dirs (TContainer directories, std::string_view const& dso_name, unsigned int dl_flags) noexcept -> void*
452455
{
453-
if (dso_name == nullptr) {
456+
if (dso_name.empty ()) {
454457
return nullptr;
455458
}
456459

@@ -469,12 +472,12 @@ auto AndroidSystem::load_dso_from_specified_dirs (TContainer directories, const
469472
return nullptr;
470473
}
471474

472-
auto AndroidSystem::load_dso_from_app_lib_dirs (const char *name, unsigned int dl_flags) noexcept -> void*
475+
auto AndroidSystem::load_dso_from_app_lib_dirs (std::string_view const& name, unsigned int dl_flags) noexcept -> void*
473476
{
474477
return load_dso_from_specified_dirs (app_lib_directories, name, dl_flags);
475478
}
476479

477-
auto AndroidSystem::load_dso_from_override_dirs (const char *name, unsigned int dl_flags) noexcept -> void*
480+
auto AndroidSystem::load_dso_from_override_dirs (std::string_view const& name, unsigned int dl_flags) noexcept -> void*
478481
{
479482
if constexpr (Constants::is_release_build) {
480483
return nullptr;
@@ -484,7 +487,7 @@ auto AndroidSystem::load_dso_from_override_dirs (const char *name, unsigned int
484487
}
485488

486489
[[gnu::flatten]]
487-
auto AndroidSystem::load_dso_from_any_directories (const char *name, unsigned int dl_flags) noexcept -> void*
490+
auto AndroidSystem::load_dso_from_any_directories (std::string_view const& name, unsigned int dl_flags) noexcept -> void*
488491
{
489492
void *handle = load_dso_from_override_dirs (name, dl_flags);
490493
if (handle == nullptr) {

0 commit comments

Comments
 (0)