CuraEngine
CuraEngine copied to clipboard
writeInt2mm: prevents OOB
In some case writeInt2mm
reads a char before the start of the buffer. This is mostly harmless as the buffer is on the stack far from protected memory but might introduce unpredictable behavior depending on the compiler.
This commits adds a check.
Separately, I wrote a new from scratch implementation of stream writers that enables changing the fixed point precision at compile time. It also speeds up the gcode writer quite a bit, which is the bottleneck on many core machines. It is still a work in progress, but if you are interested I could make a PR once edge cases are resolved.
I was looking at the code in string.h only yesterday, and felt very frustrated with all the "old" skool code over there. I started with a rough sketch how I would write something like this. Still work in progress.
#include <charconv>
#include <concepts>
#include <string_view>
#include <spdlog/spdlog.h>
template<class T>
concept Number = std::integral<T> || std::floating_point<T>;
template<Number T>
constexpr T toNumber(std::string_view word) noexcept
{
T value {};
auto [ptr, ec] { std::from_chars(word.data(), word.data() + word.length(), value) };
if (ec == std::errc::invalid_argument)
{
spdlog::error("Error: {} isn't a number! Initialized the value with {} instead.", word, value);
}
else if (ec == std::errc::result_out_of_range)
{
spdlog::error("The number {}, couldn't be converted because it is to large! Initialized the value with {} instead.", word, value);
}
return value;
}
int main()
{
using namespace std::literals;
auto x_word = "3554564564564564569340"sv;
auto x = toNumber<uint8_t>(x_word);
fmt::print("Converted to x: {}\n", x);
auto y_word = "3554564564564564569340"sv;
auto y = toNumber<int>(y_word);
fmt::print("Converted to y: {}\n", y);
auto z_word = "-122"sv;
auto z = toNumber<uint8_t>(z_word);
fmt::print("Converted to z: {}\n", z);
auto zz_word = "122"sv;
auto zz = toNumber<uint8_t>(zz_word);
fmt::print("Converted to zz: {}\n", zz);
}
output:
[2022-08-08 19:01:41.989] [error] The number 3554564564564564569340, couldn't be converted because it is to large! Initialized the value with 0 instead.
Converted to x: 0
[2022-08-08 19:01:41.989] [error] The number 3554564564564564569340, couldn't be converted because it is to large! Initialized the value with 0 instead.
Converted to y: 0
[2022-08-08 19:01:41.989] [error] Error: -122 isn't a number! Initialized the value with 0 instead.
Converted to z: 0
Converted to zz: 122
I personally am in favor of doing the conversion with from_char
because that allows me to keep it noexcept
Using string_view
ensures that it's save with overflows and they are non-owning and therefor cheap to copy. By using concepts we will still be flexible while guarding against requesting a non-number types.
What are your thoughts on this?
What are your thoughts on this?
I'm in favor of a rewrite. Tweaking the output of snprintf
is not pretty. Also it's really slow, and it create a bottleneck when the gcode is written serially (no effective parallelization).
I have already made and tested a rewrite using the approach taken by the fmt library: see here The trick is to do two digits at a time which is faster.
It's currently on top of (and depends on) commits that change the coord_t
unit system, but I can rebase it as standalone code, do some cleanup and send a PR.
@jellespijker I answered a bit too fast and didn't realize that you were talking about the conversion string -> int. :sweat_smile:
Unless I'm mistaken I don't see this functionality in string.h? Are you referring to the parsing of settings values?
Anyway, using modern C+17 conversion functions is a good point. I'll compare my implementation against std::to_char
. However, afaik, there is no standard function for converting directly an integer representing a fixed precision rational.
@Piezoid could also be that I answered a bit to fast, without actually looking.
I might want to do this stuff behind my superwide monitor instead of on my phone next time 😉
Gonna do a proper look tomorrow again.
I just get excited with all those new initiatives from people, try to clean up and modernize 😉