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

Centos7 build fix #519

Merged
merged 4 commits into from
Jun 4, 2024
Merged

Centos7 build fix #519

merged 4 commits into from
Jun 4, 2024

Conversation

neandreithal
Copy link
Contributor

@neandreithal neandreithal commented Jun 1, 2024

I've managed to get #507 working with llvm-toolset-7 on CentOS 7. Unfortunately, I can't move to devtoolset-9 as it was done in the original issue.

@danielaparker
Copy link
Owner

I appreciate the effort!

I'd like to avoid constructing strings with s.cstr(), as that would result in a large number of length recalculations, when we already know the length.

For key_type, you should replace

key_type(s->first.c_str(), get_allocator())

with

key_type(s->first.data(), s->first.size(), get_allocator())

Similarly, you should replace

 keys.emplace(key.c_str(), get_allocator());

with

 keys.emplace(key.data(), key.size(), get_allocator());

@neandreithal
Copy link
Contributor Author

Done, thanks for the feedback!

@danielaparker danielaparker merged commit f393930 into danielaparker:master Jun 4, 2024
62 checks passed
@neandreithal neandreithal deleted the centos7-build-fix branch June 5, 2024 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants