search

ethereum / c-kzg-4844

A minimal implementation of the Polynomial Commitments API for EIP-4844 and EIP-7594, written in C.
Apache License 2.0
112 stars 105 forks source link

Do not rely on calloc zeroing out allocations #474

Closed jtraglia closed 1 month ago

jtraglia commented 1 month ago

There were two spots which relied on calloc zeroing out values. I don't like this.

I left the MEMSET_TO_FF macro option to test this. Could be useful in the future. Open to removing it.

And yeah, malloc doesn't zero out allocations, but wanted to be consistent.

Also fix a random comment's wrap that I noticed.

asn-d6 commented 1 month ago

Just curious, why you dislike relying on calloc to zero out values?

Also, why do you loop and manually set arrays to FR_ZERO, over a memset? Do you dislike relying on FR_ZERO being all zeroes?

jtraglia commented 1 month ago

Just curious, why you dislike relying on calloc to zero out values?

So calloc is required by the C standard to zero out the data, so it should be perfectly safe to rely on this. But it kind of tells me that we're depending on a feature that isn't exactly clear to everyone. It's possible someone unfamiliar with C doesn't know that calloc behaves that way.

Also, why do you loop and manually set arrays to FR_ZERO, over a memset?

I think the loop is more readable than memset. I think performance difference is negligible & probably optimized away.

Do you dislike relying on FR_ZERO being all zeroes?

To small degree, yes 😄