YHK - yet another hacker news clone

Arena Allocators in C

36 points dimonomid 2 days ago 11 comments

teo_zero 1 day ago | parent

Please be aware that pointer arithmetic is not defined on void * pointers, although many compilers allow it. You had better use char * for "data".

Also, your code is prone to overflow when you add data, used, alignment and size.

And finally, the realloc function could be reworked to do nothing if new_size is <= old_size.

mrlonglong 1 day ago | parent

Why redefine the stdints?

veltas 1 hour ago | parent

They wanted shorter aliases for them to make their code clearer, I suppose.

rfgplk 28 minutes ago | parent

The one thing Rust got right was inheriting the i64/u64/i32/u32/i8/u8 naming convention for types. Far more concise and unambiguously clear. No more typing unsigned long long int or uint64_t

flohofwoe 31 minutes ago | parent

It's ok as long as the types don't leak into the public API when the code is distributed as a library. But yeah, it looks like a leftover bad habit from pre-C99 times (I suspect the author is on Windows where MSVC was very late to the C99 party).

jtari3333 27 minutes ago | parent

Typedefing uint32t to u32 etc is extremely common

indy 2 minutes ago | parent

To make them easier to type and more aesthetically pleasing

sweetjuly 1 hour ago | parent

> This will be used to align the data. For some reason, our computers are much better at dealing with data that is aligned at 8 bytes. This means that the start of each new allocation has to be at a multiple of 8 bytes.

Casting a pointer which is not aligned to its natural alignment has undefined behavior in C. It's not just an issue of unaligned accesses being slow; in practice, your program might also crash or access unexpected data on real platforms with real compiler implementations.

flohofwoe 24 minutes ago | parent

There's really no other way for a general allocation function that doesn't know the target type. I would expand the default alignment from 8 to 16 bytes though, because this is what you typically get from malloc on 64-bit targets, and a bigger 'natural alignment' should be extremely rare (maybe needed for some SIMD types, but those are outside the C standard anyway).

> It's not just an issue of unaligned accesses being slow; in practice, your program might also crash or access unexpected data on real platforms with real compiler implementations.

Unaligned accesses are fast (unless they straddle a cache line) and work perfectly well on current mainstream CPUs (and have been working fine for a long time).

E.g. I'm really not a fan of taking every piece of C code apart because it might not work on an experimental CPU design from the 1960s which the C standard theoretically supports but no current C compiler ;)

unwind 1 hour ago | parent

More obvious problems with the code as shown:

- The macros doing arithmetic (KiB, MIN etc) don't put their arguments in parentheses, that is very dangerous.

- The code assumes that int and i32 are the same size.

- Don't cast the return value of malloc(). Please Google this.

I appreciated the use of structs by value, that's a nice design.

I agree there's a risk of overflow in the arithmetic around sizes/alignment, but with a 64-bit `usize` that should be mostly theoretical.

flohofwoe 40 minutes ago | parent

> The code assumes that int and i32 are the same size.

Tbf this is extreme nitpicking. It's very unlikely that the code will ever be built on a compiler where int is not 32 bits. Replacing the sizeof(int) with sizeof(i32) is better of course, and tbh it looks like an oversight, not intentional.

> Don't cast the return value of malloc(). Please Google this.

This is required when the code should also build in C++ mode (which some weirdos seem to prefer for "improved type safety") - in any case, the explicit cast is also entirely valid C code, so not wrong, just redundant.

> The macros doing arithmetic (KiB, MIN etc) don't put their arguments in parentheses, that is very dangerous.

Agreed.

I would add though:

> Don't typedef C primitive types to your own shortcuts (like int32_t => i32), at least not when those typedefs leak into the public API, because then your i32 may collide with another library's i32.