search

msgpack / msgpack-ruby

MessagePack implementation for Ruby / msgpack.org[Ruby]
http://msgpack.org/
Apache License 2.0
758 stars 117 forks source link

Use sizeof instead of handcrafted arithmetic #321

Closed casperisfine closed 1 year ago

casperisfine commented 1 year ago

Also removing the DISABLE_UNPACKER_STACK_RMEM flag, as I don't see its value.

@peterzhu2118

casperisfine commented 1 year ago

Ah damnit, I see why it doesn't use sizeof now, it's because it need that value in the preprocessor.

I still think we can likely clean that up somehow.

casperisfine commented 1 year ago

@peterzhu2118 if you wanna have another look.

Not sure what I'm doing is really sound.

That said I'm not sure to understand in which situation that condition could be false, so maybe we'd me better to add a static assertion on msgpack_unpacker_stack_entry_t size, to ensure it's small enough that we can fit 128 entries in a 4kB page?

peterzhu2118 commented 1 year ago

This looks good.

so maybe we'd me better to add a static assertion on msgpack_unpacker_stack_entry_t size

tbh I think that this is a good idea if we never want to turn off UNPACKER_STACK_RMEM.

casperisfine commented 1 year ago

Closing in favor of https://github.com/msgpack/msgpack-ruby/pull/324