Make path extension a bit safer #30208
Conversation
| double last_x = 0.0; | ||
| double last_y = 0.0; | ||
|
|
||
| unsigned code; | ||
|
|
||
| while ((code = path.vertex(&x[0], &y[0])) != agg::path_cmd_stop) { | ||
| while ((code = path.vertex(&std::get<0>(x), &std::get<0>(y))) != agg::path_cmd_stop) { |
There was a problem hiding this comment.
I think you can still do &x[0] no? (or x.at(0) if you really want bounds checking here; this still reads better than std::get I'd say)
There was a problem hiding this comment.
std::get is compile-time checked for constants; neither x[0] nor x.at(0) are unfortunately.
There was a problem hiding this comment.
Ugh, indeed, that's a bit annoying...
| if (code == CLOSEPOLY) { | ||
| buffer += codes[4]; | ||
| buffer += std::get<4>(codes); |
| } else if (code < 5) { | ||
| size_t size = NUM_VERTICES[code]; | ||
|
|
||
| for (size_t i = 1; i < size; ++i) { | ||
| unsigned subcode = path.vertex(&x[i], &y[i]); | ||
| unsigned subcode = path.vertex(&x.at(i), &y.at(i)); |
There was a problem hiding this comment.
I don't think the compiler can safely elide the bounds check here, because it'll have trouble proving that size is small enough (I guess the "modern C++" way of ensuring that is to make NUM_VERTICES an int templated on code etc.)
There was a problem hiding this comment.
Well, x.at is the bounds-checked version, and x[i] isn't, but somehow the compiled code remains the same size either way. (Perhaps this is because the Fedora compiler has hardening enabled somewhere?)
... by replacing double pointers by fixed-size `std::array`, or a return `tuple`. With gcc (and optimization enabled?), this has no effect on code size, but gives compile-time (and better runtime) checks that there are no out-of-bounds access.
... by avoiding double pointers.
PR summary
By replacing double pointers by
std::arrayand returned tuples. AFAICT, this doesn't have any effect on code size, but ensures that several places are checked at compile time. And for now, we already know these to be correct, but this would prevent any future problems if some sizes change.PR checklist