Skip to content

Commit

Permalink
Switch WildcardExpression indices from uint32_t to size_t to avoid na…
Browse files Browse the repository at this point in the history
…rrowing conversions when interacting with string indices.
  • Loading branch information
kirkrodrigues committed Sep 9, 2024
1 parent 1bdd235 commit 5ad17c4
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 33 deletions.
2 changes: 1 addition & 1 deletion components/core/src/clp/Grep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ set<QueryInterpretation> Grep::generate_query_substring_interpretations(
continue;
}
auto possible_substr_types = get_possible_substr_types(
WildcardExpressionView(processed_search_string, begin_idx, end_idx),
WildcardExpressionView{processed_search_string, begin_idx, end_idx},
lexer
);
if (possible_substr_types.empty()) {
Expand Down
6 changes: 3 additions & 3 deletions components/core/src/clp/WildcardExpression.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "WildcardExpression.hpp"

#include <algorithm>
#include <cstdint>
#include <cstddef>
#include <string>
#include <utility>

Expand Down Expand Up @@ -55,8 +55,8 @@ WildcardExpression::WildcardExpression(std::string processed_search_string)

WildcardExpressionView::WildcardExpressionView(
WildcardExpression const& wildcard_expression,
uint32_t const begin_idx,
uint32_t const end_idx
size_t const begin_idx,
size_t const end_idx
)
: m_search_string_ptr{&wildcard_expression},
m_begin_idx{begin_idx},
Expand Down
33 changes: 16 additions & 17 deletions components/core/src/clp/WildcardExpression.hpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef CLP_WILDCARDEXPRESSION_HPP
#define CLP_WILDCARDEXPRESSION_HPP

#include <cstdint>
#include <cstddef>
#include <string>
#include <vector>

Expand All @@ -19,26 +19,25 @@ class WildcardExpression {
public:
explicit WildcardExpression(std::string processed_search_string);

[[nodiscard]] auto
substr(uint32_t const begin_idx, uint32_t const length) const -> std::string {
[[nodiscard]] auto substr(size_t const begin_idx, size_t const length) const -> std::string {
return m_processed_search_string.substr(begin_idx, length);
}

[[nodiscard]] auto length() const -> uint32_t { return m_processed_search_string.size(); }
[[nodiscard]] auto length() const -> size_t { return m_processed_search_string.size(); }

[[nodiscard]] auto get_value_is_greedy_wildcard(uint32_t const idx) const -> bool {
[[nodiscard]] auto get_value_is_greedy_wildcard(size_t const idx) const -> bool {
return m_is_greedy_wildcard[idx];
}

[[nodiscard]] auto get_value_is_non_greedy_wildcard(uint32_t const idx) const -> bool {
[[nodiscard]] auto get_value_is_non_greedy_wildcard(size_t const idx) const -> bool {
return m_is_non_greedy_wildcard[idx];
}

[[nodiscard]] auto get_value_is_escape(uint32_t const idx) const -> bool {
[[nodiscard]] auto get_value_is_escape(size_t const idx) const -> bool {
return m_is_escape[idx];
}

[[nodiscard]] auto get_value(uint32_t const idx) const -> char {
[[nodiscard]] auto get_value(size_t const idx) const -> char {
return m_processed_search_string[idx];
}

Expand All @@ -64,8 +63,8 @@ class WildcardExpressionView {
*/
WildcardExpressionView(
WildcardExpression const& wildcard_expression,
uint32_t begin_idx,
uint32_t end_idx
size_t begin_idx,
size_t end_idx
);

/**
Expand Down Expand Up @@ -95,21 +94,21 @@ class WildcardExpressionView {
[[nodiscard]] auto surrounded_by_delims_or_wildcards(log_surgeon::lexers::ByteLexer const& lexer
) const -> bool;

[[nodiscard]] auto length() const -> uint32_t { return m_end_idx - m_begin_idx; }
[[nodiscard]] auto length() const -> size_t { return m_end_idx - m_begin_idx; }

[[nodiscard]] auto get_value_is_greedy_wildcard(uint32_t const idx) const -> bool {
[[nodiscard]] auto get_value_is_greedy_wildcard(size_t const idx) const -> bool {
return m_search_string_ptr->get_value_is_greedy_wildcard(m_begin_idx + idx);
}

[[nodiscard]] auto get_value_is_non_greedy_wildcard(uint32_t const idx) const -> bool {
[[nodiscard]] auto get_value_is_non_greedy_wildcard(size_t const idx) const -> bool {
return m_search_string_ptr->get_value_is_non_greedy_wildcard(m_begin_idx + idx);
}

[[nodiscard]] auto get_value_is_escape(uint32_t const idx) const -> bool {
[[nodiscard]] auto get_value_is_escape(size_t const idx) const -> bool {
return m_search_string_ptr->get_value_is_escape(m_begin_idx + idx);
}

[[nodiscard]] auto get_value(uint32_t const idx) const -> char {
[[nodiscard]] auto get_value(size_t const idx) const -> char {
return m_search_string_ptr->get_value(m_begin_idx + idx);
}

Expand All @@ -119,8 +118,8 @@ class WildcardExpressionView {

private:
WildcardExpression const* m_search_string_ptr;
uint32_t m_begin_idx;
uint32_t m_end_idx;
size_t m_begin_idx;
size_t m_end_idx;
};
} // namespace clp

Expand Down
30 changes: 18 additions & 12 deletions components/core/tests/test-Grep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,35 +136,41 @@ TEST_CASE("SearchString", "[SearchString][schema_search]") {
}

SECTION("surrounded_by_delims_or_wildcards and starts_or_ends_with_greedy_wildcard") {
auto search_string_view1 = WildcardExpressionView(search_string, 0, search_string.length());
auto search_string_view1 = WildcardExpressionView{search_string, 0, search_string.length()};
REQUIRE(search_string_view1.surrounded_by_delims_or_wildcards(lexer));
REQUIRE(search_string_view1.starts_or_ends_with_greedy_wildcard());
auto search_string_view2 = WildcardExpressionView(search_string, 1, search_string.length());
auto search_string_view2 = WildcardExpressionView{search_string, 1, search_string.length()};
REQUIRE(search_string_view2.surrounded_by_delims_or_wildcards(lexer));
REQUIRE(search_string_view2.starts_or_ends_with_greedy_wildcard());
auto search_string_view3 = WildcardExpressionView(search_string, 0, search_string.length() - 1);
auto search_string_view3
= WildcardExpressionView{search_string, 0, search_string.length() - 1};
REQUIRE(search_string_view3.surrounded_by_delims_or_wildcards(lexer));
REQUIRE(search_string_view3.starts_or_ends_with_greedy_wildcard());
auto search_string_view4 = WildcardExpressionView(search_string, 2, search_string.length() - 2);
auto search_string_view4
= WildcardExpressionView{search_string, 2, search_string.length() - 2};
REQUIRE(search_string_view4.surrounded_by_delims_or_wildcards(lexer));
REQUIRE(false == search_string_view4.starts_or_ends_with_greedy_wildcard());
auto search_string_view5 = WildcardExpressionView(search_string, 3, search_string.length() - 3);
auto search_string_view5
= WildcardExpressionView{search_string, 3, search_string.length() - 3};
REQUIRE(false == search_string_view5.surrounded_by_delims_or_wildcards(lexer));
REQUIRE(false == search_string_view5.starts_or_ends_with_greedy_wildcard());
auto search_string_view6 = WildcardExpressionView(search_string, 1, search_string.length() - 1);
auto search_string_view6
= WildcardExpressionView{search_string, 1, search_string.length() - 1};
REQUIRE(search_string_view6.surrounded_by_delims_or_wildcards(lexer));
REQUIRE(false == search_string_view6.starts_or_ends_with_greedy_wildcard());
}

SECTION("extend_to_adjacent_greedy_wildcards") {
auto search_string_view = WildcardExpressionView(search_string, 1, search_string.length() - 1);
auto search_string_view
= WildcardExpressionView{search_string, 1, search_string.length() - 1};
REQUIRE(8 == search_string_view.length());
auto extended_search_string_view = search_string_view.extend_to_adjacent_greedy_wildcards();
REQUIRE(extended_search_string_view.surrounded_by_delims_or_wildcards(lexer));
REQUIRE(10 == extended_search_string_view.length());
REQUIRE(extended_search_string_view.get_substr_copy() == "* test\\* *");

auto search_string_view2 = WildcardExpressionView(search_string, 2, search_string.length() - 2);
auto search_string_view2
= WildcardExpressionView{search_string, 2, search_string.length() - 2};
REQUIRE(6 == search_string_view2.length());
auto extended_search_string_view2
= search_string_view2.extend_to_adjacent_greedy_wildcards();
Expand All @@ -174,7 +180,7 @@ TEST_CASE("SearchString", "[SearchString][schema_search]") {
}

SECTION("getters") {
auto search_string_view = WildcardExpressionView(search_string, 2, search_string.length());
auto search_string_view = WildcardExpressionView{search_string, 2, search_string.length()};
REQUIRE(false == search_string_view.is_greedy_wildcard());
REQUIRE(false == search_string_view.is_non_greedy_wildcard());
REQUIRE('t' == search_string_view.get_value(0));
Expand All @@ -196,7 +202,7 @@ TEST_CASE("SearchString", "[SearchString][schema_search]") {
}

SECTION("Greedy Wildcard") {
auto search_string_view = WildcardExpressionView(search_string, 0, 1);
auto search_string_view = WildcardExpressionView{search_string, 0, 1};
REQUIRE(search_string_view.is_greedy_wildcard());
REQUIRE(false == search_string_view.is_non_greedy_wildcard());
}
Expand All @@ -213,7 +219,7 @@ TEST_CASE("get_substring_variable_types", "[get_substring_variable_types][schema
for (uint32_t end_idx = 1; end_idx <= search_string.length(); end_idx++) {
for (uint32_t begin_idx = 0; begin_idx < end_idx; begin_idx++) {
auto [variable_types, contains_wildcard] = Grep::get_substring_variable_types(
WildcardExpressionView(search_string, begin_idx, end_idx),
WildcardExpressionView{search_string, begin_idx, end_idx},
lexer
);
std::set<uint32_t> expected_variable_types;
Expand Down Expand Up @@ -263,7 +269,7 @@ TEST_CASE("get_possible_substr_types", "[get_possible_substr_types][schema_searc
for (uint32_t end_idx = 1; end_idx <= search_string.length(); end_idx++) {
for (uint32_t begin_idx = 0; begin_idx < end_idx; begin_idx++) {
auto query_logtypes = Grep::get_possible_substr_types(
WildcardExpressionView(search_string, begin_idx, end_idx),
WildcardExpressionView{search_string, begin_idx, end_idx},
lexer
);
vector<QueryInterpretation> expected_result(0);
Expand Down

0 comments on commit 5ad17c4

Please sign in to comment.