search

LLNL / zfp

Compressed numerical arrays that support high-speed random access
http://zfp.llnl.gov
BSD 3-Clause "New" or "Revised" License
754 stars 152 forks source link

Better argument typing #223

Closed mirounga closed 6 months ago

mirounga commented 6 months ago

Consistent argument types for lift functions.

lindstro commented 6 months ago

@mirounga I agree with replacing uint with ptrdiff_t. But what do the pointer casts accomplish? Such casts are generally considered poor style in C. Moreover, they may silently introduce bugs such as casting away const qualifiers that would be caught without the (unnecessary) cast.

mirounga commented 6 months ago

Currently some functions have casts and some do not. In my experiments I have sometimes compiled the code as C++, and the absence of the cast had caused errors. If you consider casts bad idea, I could easily revert them.

lindstro commented 6 months ago

For consistency and adherence to best practice, I would like to avoid any unnecessary casts involving void*. If there are such casts elsewhere, can you please point them out? I noticed that there are a few involving malloc() that were added over time for no obvious (to me) reason.

I really do not want to take on the burden of ensuring that the zfp C library can also be compiled with a C++ compiler without a compelling reason. What is your reason for needing this, and are there not other changes that would be required?

Since this would be the first commit to develop following our last release, we'd also have to flag this as a development version. Because the PR amounts to only two changed lines of code, it may be easiest if I simply take care of the necessary changes myself. I do appreciate you pointing them out, however.