Changes to prepare for ubuntu 22.04 CI #6635
Conversation
Pentarctagon
force-pushed
the
master
branch
2 times, most recently
from
8a8b3c4
to
8b14c3d
Compare
github-actions
bot
added
the
UI
Pentarctagon
force-pushed
the
master
branch
3 times, most recently
from
e977d3c
to
44b12a5
Compare
|
The "Fix range-loop-construct warning" commit should be done by using either the |
|
The what? |
|
If you want a string array: using namespace std::string_literals;
static const std::array required {"id"s, "config_name"s, "create_map"s};If you want a string_view array: using namespace std::string_view_literals
static const std::array required {"id"sv, "config_name"sv, "create_map"sv};Right now the compiler is deducing an array of |
Pentarctagon
force-pushed
the
master
branch
2 times, most recently
from
35efe6b
to
74ccb90
Compare
It was used in once place and openssl deprecated the current APIs used, so just remove it rather than update it.
Pentarctagon
changed the title
|
|
||
| static_assert(utils::md5::DIGEST_SIZE == MD5_DIGEST_LENGTH, "Constants mismatch"); | ||
| static_assert(utils::sha1::DIGEST_SIZE == SHA_DIGEST_LENGTH, "Constants mismatch"); | ||
| #include <openssl/evp.h> |
There was a problem hiding this comment.
Why did you remove the MD5 static_assert here but not in the Apple section? You should probably remove either both or neither.
There was a problem hiding this comment.
Because the new evp header doesn't have the constant to compare against anymore, and the evp header isn't used on Apple systems.
There was a problem hiding this comment.
Well… how does it even know what the size is? Is it just hard-coded and will break if we ever change hash type?
Although, I guess if we change hash type we'd at least make a separate hashing class, so maybe that doesn't matter…
There was a problem hiding this comment.
I don't follow? Are there multiple types of MD5 hashes?
| // TODO: use EVP_EncryptInit_ex2 once openssl 3.0 is more widespread | ||
| EVP_EncryptInit_ex(ctx, EVP_rc4(), NULL, key.data(), NULL); | ||
| EVP_EncryptUpdate(ctx, result.data(), &outlen, text.data(), text.size()); | ||
| EVP_EncryptFinal_ex(ctx, result.data() + outlen, &tmplen); |
There was a problem hiding this comment.
Did you check that outlen will always be equal to the input length?
It seems like a plausible assumption but I'm not sure if it's a correct one.
There was a problem hiding this comment.
I don't follow. The output length is a separate value from the input length?
There was a problem hiding this comment.
Yeah? I'm not sure how RC4 works, but some encryption schemes require padding to bring the ciphertext to a multiple of the block size.
There was a problem hiding this comment.
The previous implementation doesn't seem to, at least. It calls RC4 with a length of block_size as many times as it can, then calls it again at the end with the size of any remaining characters that aren't enough to fill a block. So there's no padding being done.
| @@ -623,7 +623,8 @@ class dispatcher | |||
| } else if constexpr(cat == event_category::text_input) { | |||
| return signal_text_input_queue_; | |||
| } else { | |||
| static_assert(utils::dependent_false_v<cat>, "No matching signal queue for category"); | |||
| // "No matching signal queue for category" | |||
| utils::static_assert_false(); | |||
There was a problem hiding this comment.
This is a negative change, as it loses the error message…
There was a problem hiding this comment.
It was either this or remove it entirely, since Clang was rejecting the version that allowed an error message.
There was a problem hiding this comment.
Sounds like a clang bug. You're supposed to be able to use a message. Or did it have a problem with dependent_false_v?
There was a problem hiding this comment.
There was a problem hiding this comment.
That looks like it could have been solved with decltype(cat).
No description provided.