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.

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

Migrate kUnstoppableDomains and a few other string arrays to proper containers #42484

Open
cdesouza-chromium opened this issue Nov 23, 2024 · 0 comments · May be fixed by brave/brave-core#26710

Comments

@cdesouza-chromium
Copy link
Contributor

Description

These are the few remaining cases of array of strings that can be modernised.

@cdesouza-chromium cdesouza-chromium self-assigned this Nov 23, 2024
cdesouza-chromium added a commit to brave/brave-core that referenced this issue Nov 23, 2024
This PR changes a few places where we used to have we had something
like:

```cxx
constexpr const char* kConstant[] = ...
```

These arrays array are not as safe as proper containers. There's also
the challange that the strings referred by these arrays cannot have
their sizes encoded on the type, so there's a certain reliance on
`strlen` to determine the size.

This change converts on of these arrays to `std::array`. The other array
was converted to a `constexpr` flat set, as that is what this array was
being used for. This flat set requires a custom comparator though, as
the search of the elements as matching against `end_with`. With this
change, the lookup will be a bit less wasteful.

Resolves brave/brave-browser#42484
cdesouza-chromium added a commit to brave/brave-core that referenced this issue Nov 23, 2024
This PR changes a few places where we used to have we had something
like:

```cxx
constexpr const char* kConstant[] = ...
```

These arrays array are not as safe as proper containers. There's also
the challange that the strings referred by these arrays cannot have
their sizes encoded on the type, so there's a certain reliance on
`strlen` to determine the size.

This change converts on of these arrays to `std::array`. The other array
was converted to a `constexpr` flat set, as that is what this array was
being used for. This flat set requires a custom comparator though, as
the search of the elements as matching against `end_with`. With this
change, the lookup will be a bit less wasteful.

Resolves brave/brave-browser#42484
cdesouza-chromium added a commit to brave/brave-core that referenced this issue Nov 24, 2024
This PR changes a few places where we used to have we had something
like:

```cxx
constexpr const char* kConstant[] = ...
```

These arrays array are not as safe as proper containers. There's also
the challange that the strings referred by these arrays cannot have
their sizes encoded on the type, so there's a certain reliance on
`strlen` to determine the size.

This change converts on of these arrays to `std::array`. The other array
was converted to a `constexpr` flat set, as that is what this array was
being used for. This flat set requires a custom comparator though, as
the search of the elements as matching against `end_with`. With this
change, the lookup will be a bit less wasteful.

Resolves brave/brave-browser#42484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant