3 ms·
DON'T USE THIS As fpoling points out, capacity is a 32-bit unsigned. That can overflow. There is no safety in append: if capacity is zero no new space will b
by klyrs 4y ago
DON'T USE THIS
As fpoling points out, capacity is a 32-bit unsigned. That can overflow. There is no safety in append: if capacity is zero no new space will be made, if capacity overflows the realloc will not have enough space -- in either case, you end up writing past the end.
Allocating a [capacity] zero array and appending to it is extremely common. That you'll write past the end in that case shows that the author has barely even used this.
The code is unacceptably bad and unsafe. I don't usually do this, but I'm going to flag this post. I encourage everybody to do the same.
Apologies to the author. Golf is fun. This is uncool.
edit: changed size to [capacity]
- nice_byte 4y agoI have been using this for years. Capacity is not size. A zero size array will have non zero capacity. All c code is unsafe.
- klyrs 4y agoWhere do you check that capacity is nonzero? When capacity is zero, what happens on this line? a.capacity <<= 1u; "all c code is unsafe" is not an excuse to permit bloody obvious, undocumented memory overruns. I write a lot of c. Avoiding the unsafe bits, avoiding UB, is the skill required to write good c. "C code is unsafe" is a Rustacean marketing slogan. Don't believe it, but definitely don't practice it.
- nice_byte 4y agojust don't initialize it with 0 capacity :-)
- skullt 4y agoWhy not just fix it? It's a trivial fix and has the added benefit that a zero-initialized DYN_ARR_OF(x) struct would be in a valid state, which is always nice. A struct with several dynamic arrays can then much simpler to initialize, for example.
- acmj 4y agoFor a library, if the user input is wrong, throw an error or an assertion failure. It shouldn't be causing an uninformative segmentation fault which your library does. The fix is a trivial. Why so defensive?
- marginalia_nu 4y agoIs this overflow really a realistic scenario? I imagine most machines won't even permit you to allocate the sorts of sizes that would cause this bug. In that scenario (where you're allocating a multi-gigabytes array), you typically know the size ahead of time, instead of growing it dynamically, as the latter doesn't really perform too well.
- klyrs 4y ago2Gb (1<<31 + 1 = boom) just isn't that much memory these days. If you're parsing large text files, for example, it's really easy to blow that limit. And what's the point of this dynamically-growing array if not to forget about managing capacity?
- deleted 4y ago[deleted]