Skip to content

Commit

Permalink
Everywhere: Explicitly specify the size in StringView constructors
Browse files Browse the repository at this point in the history
This commit moves the length calculations out to be directly on the
StringView users. This is an important step towards the goal of removing
StringView(char const*), as it moves the responsibility of calculating
the size of the string to the user of the StringView (which will prevent
naive uses causing OOB access).
  • Loading branch information
sin-ack authored and awesomekling committed Jul 12, 2022
1 parent e3da0ad commit c70f45f
Show file tree
Hide file tree
Showing 75 changed files with 264 additions and 203 deletions.
2 changes: 1 addition & 1 deletion AK/Demangle.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ inline String demangle(StringView name)
{
int status = 0;
auto* demangled_name = abi::__cxa_demangle(name.to_string().characters(), nullptr, nullptr, &status);
auto string = String(status == 0 ? demangled_name : name);
auto string = String(status == 0 ? StringView { demangled_name, strlen(demangled_name) } : name);
if (status == 0)
free(demangled_name);
return string;
Expand Down
2 changes: 1 addition & 1 deletion AK/GenericLexer.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class GenericLexer {

constexpr bool consume_specific(char const* next)
{
return consume_specific(StringView { next });
return consume_specific(StringView { next, __builtin_strlen(next) });
}

constexpr char consume_escaped_character(char escape_char = '\\', StringView escape_map = "n\nr\rt\tb\bf\f")
Expand Down
4 changes: 2 additions & 2 deletions AK/JsonObjectSerializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ class JsonObjectSerializer {
TRY(begin_item(key));
if constexpr (IsLegacyBuilder<Builder>) {
TRY(m_builder.try_append('"'));
TRY(m_builder.try_append_escaped_for_json(value));
TRY(m_builder.try_append_escaped_for_json({ value, strlen(value) }));
TRY(m_builder.try_append('"'));
} else {
TRY(m_builder.append('"'));
TRY(m_builder.append_escaped_for_json(value));
TRY(m_builder.append_escaped_for_json({ value, strlen(value) }));
TRY(m_builder.append('"'));
}
return {};
Expand Down
4 changes: 2 additions & 2 deletions AK/SourceLocation.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace AK {

class SourceLocation {
public:
[[nodiscard]] constexpr StringView function_name() const { return StringView(m_function); }
[[nodiscard]] constexpr StringView filename() const { return StringView(m_file); }
[[nodiscard]] constexpr StringView function_name() const { return { m_function, __builtin_strlen(m_function) }; }
[[nodiscard]] constexpr StringView filename() const { return { m_file, __builtin_strlen(m_file) }; }
[[nodiscard]] constexpr u32 line_number() const { return m_line; }

[[nodiscard]] static constexpr SourceLocation current(char const* const file = __builtin_FILE(), u32 line = __builtin_LINE(), char const* const function = __builtin_FUNCTION())
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Arch/x86/common/Processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ UNMAP_AFTER_INIT void Processor::detect_hypervisor_hyperv(CPUID const& hyperviso
alignas(sizeof(u32)) char interface_signature_buffer[5];
*reinterpret_cast<u32*>(interface_signature_buffer) = hypervisor_interface.eax();
interface_signature_buffer[4] = '\0';
StringView hyperv_interface_signature(interface_signature_buffer);
StringView hyperv_interface_signature { interface_signature_buffer, strlen(interface_signature_buffer) };

dmesgln("CPU[{}]: Hyper-V interface signature '{}' ({:#x})", current_id(), hyperv_interface_signature, hypervisor_interface.eax());

Expand Down
2 changes: 1 addition & 1 deletion Kernel/CommandLine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ CommandLine const& kernel_command_line()
UNMAP_AFTER_INIT void CommandLine::initialize()
{
VERIFY(!s_the);
s_the = new CommandLine(s_cmd_line);
s_the = new CommandLine({ s_cmd_line, strlen(s_cmd_line) });
dmesgln("Kernel Commandline: {}", kernel_command_line().string());
// Validate the modes the user passed in.
(void)s_the->panic_mode(Validate::Yes);
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Net/IPv4Socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ ErrorOr<void> IPv4Socket::ioctl(OpenFileDescription&, unsigned request, Userspac
memcpy(namebuf, ifr.ifr_name, IFNAMSIZ);
namebuf[sizeof(namebuf) - 1] = '\0';

auto adapter = NetworkingManagement::the().lookup_by_name(namebuf);
auto adapter = NetworkingManagement::the().lookup_by_name({ namebuf, strlen(namebuf) });
if (!adapter)
return ENODEV;

Expand Down
2 changes: 1 addition & 1 deletion Kernel/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ UNMAP_AFTER_INIT void setup_serial_debug()
// serial_debug will output all the dbgln() data to COM1 at
// 8-N-1 57600 baud. this is particularly useful for debugging the boot
// process on live hardware.
if (StringView(kernel_cmdline).contains("serial_debug")) {
if (StringView { kernel_cmdline, strlen(kernel_cmdline) }.contains("serial_debug"sv)) {
set_serial_debug(true);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ int main(int argc, char** argv)
.short_name = 'i',
.value_name = "path",
.accept_value = [&](char const* s) {
s_header_search_paths.append(s);
s_header_search_paths.append({ s, strlen(s) });
return true;
},
});
Expand Down
2 changes: 1 addition & 1 deletion Tests/AK/TestSourceLocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ static StringView test_default_arg(SourceLocation const& loc = SourceLocation::c
TEST_CASE(default_arg_scenario)
{
auto actual_calling_function = test_default_arg();
auto expected_calling_function = StringView(__FUNCTION__);
auto expected_calling_function = StringView { __FUNCTION__, strlen(__FUNCTION__) };

EXPECT_EQ(expected_calling_function, actual_calling_function);
}
2 changes: 1 addition & 1 deletion Tests/AK/TestStringView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ TEST_CASE(construct_empty)
TEST_CASE(view_literal)
{
char const* truth = "cats rule dogs drool";
StringView view(truth);
StringView view { truth, strlen(truth) };
EXPECT_EQ(view.is_null(), false);
EXPECT_EQ(view.characters_without_null_termination(), truth);
EXPECT_EQ(view, view);
Expand Down
40 changes: 20 additions & 20 deletions Tests/AK/TestUtf8.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,33 +51,33 @@ TEST_CASE(decode_utf8)
TEST_CASE(validate_invalid_ut8)
{
size_t valid_bytes;
char invalid_utf8_1[] = { 42, 35, (char)182, 9, 0 };
Utf8View utf8_1 { StringView { invalid_utf8_1 } };
char invalid_utf8_1[] = { 42, 35, (char)182, 9 };
Utf8View utf8_1 { StringView { invalid_utf8_1, 4 } };
EXPECT(!utf8_1.validate(valid_bytes));
EXPECT(valid_bytes == 2);

char invalid_utf8_2[] = { 42, 35, (char)208, (char)208, 0 };
Utf8View utf8_2 { StringView { invalid_utf8_2 } };
char invalid_utf8_2[] = { 42, 35, (char)208, (char)208 };
Utf8View utf8_2 { StringView { invalid_utf8_2, 4 } };
EXPECT(!utf8_2.validate(valid_bytes));
EXPECT(valid_bytes == 2);

char invalid_utf8_3[] = { (char)208, 0 };
Utf8View utf8_3 { StringView { invalid_utf8_3 } };
char invalid_utf8_3[] = { (char)208 };
Utf8View utf8_3 { StringView { invalid_utf8_3, 1 } };
EXPECT(!utf8_3.validate(valid_bytes));
EXPECT(valid_bytes == 0);

char invalid_utf8_4[] = { (char)208, 35, 0 };
Utf8View utf8_4 { StringView { invalid_utf8_4 } };
char invalid_utf8_4[] = { (char)208, 35 };
Utf8View utf8_4 { StringView { invalid_utf8_4, 2 } };
EXPECT(!utf8_4.validate(valid_bytes));
EXPECT(valid_bytes == 0);

char invalid_utf8_5[] = { (char)0xf4, (char)0x8f, (char)0xbf, (char)0xc0, 0 }; // U+110000
Utf8View utf8_5 { StringView { invalid_utf8_5 } };
char invalid_utf8_5[] = { (char)0xf4, (char)0x8f, (char)0xbf, (char)0xc0 }; // U+110000
Utf8View utf8_5 { StringView { invalid_utf8_5, 4 } };
EXPECT(!utf8_5.validate(valid_bytes));
EXPECT(valid_bytes == 0);

char invalid_utf8_6[] = { (char)0xf4, (char)0xa1, (char)0xb0, (char)0xbd, 0 }; // U+121c3d
Utf8View utf8_6 { StringView { invalid_utf8_6 } };
char invalid_utf8_6[] = { (char)0xf4, (char)0xa1, (char)0xb0, (char)0xbd }; // U+121c3d
Utf8View utf8_6 { StringView { invalid_utf8_6, 4 } };
EXPECT(!utf8_6.validate(valid_bytes));
EXPECT(valid_bytes == 0);
}
Expand Down Expand Up @@ -122,8 +122,8 @@ TEST_CASE(decode_invalid_ut8)
{
// Test case 1 : Getting an extension byte as first byte of the code point
{
char raw_data[] = { 'a', 'b', (char)0xA0, 'd', 0 };
Utf8View view { StringView { raw_data } };
char raw_data[] = { 'a', 'b', (char)0xA0, 'd' };
Utf8View view { StringView { raw_data, 4 } };
u32 expected_characters[] = { 'a', 'b', 0xFFFD, 'd' };
String expected_underlying_bytes[] = { "a", "b", "\xA0", "d" };
size_t expected_size = sizeof(expected_characters) / sizeof(expected_characters[0]);
Expand All @@ -140,8 +140,8 @@ TEST_CASE(decode_invalid_ut8)

// Test case 2 : Getting a non-extension byte when an extension byte is expected
{
char raw_data[] = { 'a', 'b', (char)0xC0, 'd', 'e', 0 };
Utf8View view { StringView { raw_data } };
char raw_data[] = { 'a', 'b', (char)0xC0, 'd', 'e' };
Utf8View view { StringView { raw_data, 5 } };
u32 expected_characters[] = { 'a', 'b', 0xFFFD, 'd', 'e' };
String expected_underlying_bytes[] = { "a", "b", "\xC0", "d", "e" };
size_t expected_size = sizeof(expected_characters) / sizeof(expected_characters[0]);
Expand All @@ -158,8 +158,8 @@ TEST_CASE(decode_invalid_ut8)

// Test case 3 : Not enough bytes before the end of the string
{
char raw_data[] = { 'a', 'b', (char)0x90, 'd', 0 };
Utf8View view { StringView { raw_data } };
char raw_data[] = { 'a', 'b', (char)0x90, 'd' };
Utf8View view { StringView { raw_data, 4 } };
u32 expected_characters[] = { 'a', 'b', 0xFFFD, 'd' };
String expected_underlying_bytes[] = { "a", "b", "\x90", "d" };
size_t expected_size = sizeof(expected_characters) / sizeof(expected_characters[0]);
Expand All @@ -176,8 +176,8 @@ TEST_CASE(decode_invalid_ut8)

// Test case 4 : Not enough bytes at the end of the string
{
char raw_data[] = { 'a', 'b', 'c', (char)0x90, 0 };
Utf8View view { StringView { raw_data } };
char raw_data[] = { 'a', 'b', 'c', (char)0x90 };
Utf8View view { StringView { raw_data, 4 } };
u32 expected_characters[] = { 'a', 'b', 'c', 0xFFFD };
String expected_underlying_bytes[] = { "a", "b", "c", "\x90" };
size_t expected_size = sizeof(expected_characters) / sizeof(expected_characters[0]);
Expand Down
8 changes: 4 additions & 4 deletions Tests/LibC/TestLibCTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ TEST_CASE(asctime)

time_t epoch = 0;
auto result = asctime(localtime(&epoch));
EXPECT_EQ(expected_epoch, StringView(result));
EXPECT_EQ(expected_epoch, StringView(result, strlen(result)));
}

TEST_CASE(asctime_r)
Expand All @@ -51,7 +51,7 @@ TEST_CASE(asctime_r)
char buffer[26] {};
time_t epoch = 0;
auto result = asctime_r(localtime(&epoch), buffer);
EXPECT_EQ(expected_epoch, StringView(result));
EXPECT_EQ(expected_epoch, StringView(result, strlen(result)));
}

TEST_CASE(ctime)
Expand All @@ -61,7 +61,7 @@ TEST_CASE(ctime)
time_t epoch = 0;
auto result = ctime(&epoch);

EXPECT_EQ(expected_epoch, StringView(result));
EXPECT_EQ(expected_epoch, StringView(result, strlen(result)));
}

TEST_CASE(ctime_r)
Expand All @@ -72,7 +72,7 @@ TEST_CASE(ctime_r)
time_t epoch = 0;
auto result = ctime_r(&epoch, buffer);

EXPECT_EQ(expected_epoch, StringView(result));
EXPECT_EQ(expected_epoch, StringView(result, strlen(result)));
}

TEST_CASE(tzset)
Expand Down
4 changes: 2 additions & 2 deletions Tests/LibC/TestRealpath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ TEST_CASE(overlong_realpath)

// Then, create a long path.
StringBuilder expected;
expected.append(tmp_dir);
expected.append({ tmp_dir, strlen(tmp_dir) });

// But first, demonstrate the functionality at a reasonable depth:
auto expected_str = expected.build();
Expand All @@ -63,7 +63,7 @@ TEST_CASE(overlong_realpath)
return;
}
expected.append('/');
expected.append(PATH_LOREM_250);
expected.append({ PATH_LOREM_250, strlen(PATH_LOREM_250) });
ret = chdir(PATH_LOREM_250);
if (ret < 0) {
perror("chdir iter");
Expand Down
10 changes: 6 additions & 4 deletions Userland/Applications/Help/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

using namespace Help;

static String parse_input(char const* input)
static String parse_input(StringView input)
{
AK::URL url(input);
if (url.is_valid())
Expand Down Expand Up @@ -50,9 +50,10 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
.name = "section",
.min_values = 0,
.max_values = 1,
.accept_value = [&](char const* input) {
.accept_value = [&](char const* input_ptr) {
StringView input { input_ptr, strlen(input_ptr) };
// If it's a number, use it as the section
if (auto number = StringView(input).to_int(); number.has_value()) {
if (auto number = input.to_int(); number.has_value()) {
section = number.value();
return true;
}
Expand All @@ -66,7 +67,8 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
.name = "page",
.min_values = 0,
.max_values = 1,
.accept_value = [&](char const* input) {
.accept_value = [&](char const* input_ptr) {
StringView input { input_ptr, strlen(input_ptr) };
// If start_page was already set by our section arg, then it can't be set again
if (start_page.is_empty())
return false;
Expand Down
3 changes: 2 additions & 1 deletion Userland/Applications/SpaceAnalyzer/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ static void analyze(RefPtr<Tree> tree, SpaceAnalyzer::TreeMapWidget& treemapwidg
if (!first) {
builder.append(", ");
}
builder.append(strerror(key));
auto const* error = strerror(key);
builder.append({ error, strlen(error) });
builder.append(" (");
int value = error_accumulator.get(key).value();
builder.append(String::number(value));
Expand Down
4 changes: 3 additions & 1 deletion Userland/Applications/Spreadsheet/ExportDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,11 @@ Result<void, String> ExportDialog::make_and_run_for(StringView mime, Core::File&
bool result = file.write(file_content);
if (!result) {
int error_number = errno;
auto const* error = strerror(error_number);

StringBuilder sb;
sb.append("Unable to save file. Error: ");
sb.append(strerror(error_number));
sb.append({ error, strlen(error) });

return sb.to_string();
}
Expand Down
6 changes: 5 additions & 1 deletion Userland/DevTools/HackStudio/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ static void notify_make_not_available()
static void update_path_environment_variable()
{
StringBuilder path;
path.append(getenv("PATH"));

auto const* path_env_ptr = getenv("PATH");
if (path_env_ptr != NULL)
path.append({ path_env_ptr, strlen(path_env_ptr) });

if (path.length())
path.append(":");
path.append("/usr/local/sbin:/usr/local/bin:/usr/bin:/bin");
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibArchive/Tar.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class [[gnu::packed]] TarFileHeader {
time_t timestamp() const { return get_field_as_integral(m_timestamp); }
unsigned checksum() const { return get_field_as_integral(m_checksum); }
TarFileType type_flag() const { return TarFileType(m_type_flag); }
StringView link_name() const { return m_link_name; }
StringView link_name() const { return { m_link_name, strlen(m_link_name) }; }
StringView magic() const { return get_field_as_string_view(m_magic); }
StringView version() const { return get_field_as_string_view(m_version); }
StringView owner_name() const { return get_field_as_string_view(m_owner_name); }
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibC/arpa/inet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ int inet_pton(int af, char const* src, void* dst)
*(uint32_t*)dst = u.l;
return 1;
} else if (af == AF_INET6) {
auto addr = IPv6Address::from_string(src);
auto addr = IPv6Address::from_string({ src, strlen(src) });
if (!addr.has_value()) {
errno = EINVAL;
return 0;
Expand Down
8 changes: 4 additions & 4 deletions Userland/Libraries/LibC/getopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,11 @@ int OptionParser::handle_short_option()

option const* OptionParser::lookup_long_option(char* raw) const
{
StringView arg = raw;
StringView arg { raw, strlen(raw) };

for (size_t index = 0; m_long_options[index].name; index++) {
auto& option = m_long_options[index];
StringView name = option.name;
StringView name { option.name, strlen(option.name) };

if (!arg.starts_with(name))
continue;
Expand Down Expand Up @@ -347,12 +347,12 @@ bool OptionParser::find_next_option()
int getopt(int argc, char* const* argv, char const* short_options)
{
option dummy { nullptr, 0, nullptr, 0 };
OptionParser parser { argc, argv, short_options, &dummy };
OptionParser parser { argc, argv, { short_options, strlen(short_options) }, &dummy };
return parser.getopt();
}

int getopt_long(int argc, char* const* argv, char const* short_options, const struct option* long_options, int* out_long_option_index)
{
OptionParser parser { argc, argv, short_options, long_options, out_long_option_index };
OptionParser parser { argc, argv, { short_options, strlen(short_options) }, long_options, out_long_option_index };
return parser.getopt();
}
6 changes: 4 additions & 2 deletions Userland/Libraries/LibC/getsubopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ int getsubopt(char** option_array, char* const* tokens, char** option_value)
if (**option_array == '\0')
return -1;

auto option_string = StringView(*option_array);
auto const* option_ptr = *option_array;
StringView option_string { option_ptr, strlen(option_ptr) };

auto possible_comma_location = option_string.find(',');
char* option_end = const_cast<char*>(option_string.characters_without_null_termination()) + possible_comma_location.value_or(option_string.length());
Expand All @@ -34,7 +35,8 @@ int getsubopt(char** option_array, char* const* tokens, char** option_value)
});

for (int count = 0; tokens[count] != NULL; ++count) {
auto token_stringview = StringView(tokens[count]);
auto const* token = tokens[count];
StringView token_stringview { token, strlen(token) };
if (!option_string.starts_with(token_stringview))
continue;
if (tokens[count][value_start - *option_array] != '\0')
Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibC/netdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ hostent* gethostbyname(char const* name)
{
h_errno = 0;

auto ipv4_address = IPv4Address::from_string(name);
auto ipv4_address = IPv4Address::from_string({ name, strlen(name) });

if (ipv4_address.has_value()) {
gethostbyname_name_buffer = ipv4_address.value().to_string();
Expand Down
Loading

0 comments on commit c70f45f

Please sign in to comment.