Fix many warnings in libnczarr #2852
Conversation
More consistent with similar structs, and generates fewer warnings
Some mix-up here: `mode_t` was used for some netCDF-specific modes, and `int` used for some POSIX-specific modes
Try to be more consistent in use of `size_t` across internal library declarations
There was a problem hiding this comment.
- ncconfigure.h is normally included by including config.h.
config.h should be in zincludes.h - zinfo->controls.flags &= (size64_t)(~noflags);
should this be zinfo->controls.flags &= ~((size64_t)(noflags));
There was a problem hiding this comment.
-
Ah sorry, my editor automatically includes the most relevant header when inserting new types. It looks like nothing in
zincludes.his used directly which is probably why it chosencconfigure.hinstead. -
It's probably actually clearer to change the type of
noflagsitself, not sure why I didn't spot that, thanks
There was a problem hiding this comment.
You will note that as a rule, we always declare variables at the beginning of a block e.g. {...}
There used to be compilers that complained about mid-code declarations. Not sure if that
is still true, but I still adhere to that convention.
For similar reasons, I never declare for-loop iterators in the loop e.g. I never use for(size_t i...).
There was a problem hiding this comment.
Yes, this is a limitation of C89 and relaxed in C99. It's supported by every compiler on Compiler Explorer (except for a 6502 specific compiler and one for TI calculators).
I would strongly suggest moving to this style when writing new code, as it can avoid bugs by reducing the scope of variables and preventing use of uninitialised values.
|
Changes to the NCZarr code may be a waste of time. I am in the process of adding |
|
The CMake debug build enables: |
`loaded_plugins_max` is *not* the total number of loaded plugins, but the max ID, and defaults to `-1`. This clashes with the change of the index to `size_t`
|
@DennisHeimbigner GitHub isn't showing what you're commenting on, but if it's in reference to f8da95a, then it's due to implicit conversions:
|
* main: (248 commits) Updated Release Notes. A bit of cleanup for now, more to follow, but moving on to other roadblocks. Comment out debugging messages, will remove before final merge. Clean up the logic, remove some rough edges. Add a filter to process options and report deprecation warning. Remove dangling define in netcdf_meta.h template file. No effect other than to remove a personal annoyance I introduced in the first place. Updated the nc-config script with the following: adding deprecation error for usage of NETCDF_ENABLE_NETCDF4 removing cmake alias variable for netcdf4 removing script replacing something that removes semicolons another one fixing some that were missing? removing variables Replacing NC_USE_STATIC_CRT with NETCDF_USE_STATIC_CRT Replace NC_FIND_SHARED_LIBS with NETCDF_FIND_SHARED_LIBS Replace ENABLE_XGETOPT with NETCDF_ENABLE_XGETOPT Replace ENABLE_UNIT_TESTS with NETCDF_ENABLE_UNIT_TESTS Replace ENABLE_TESTS with NETCDF_ENABLE_TESTS Replace ENABLE_STRICT_NULL_BYTE_HEADER_PADDING with NETCDF_ENABLE_STRICT_NULL_BYTE_HEADER_PADDING ...
Also delete actually unused variable
|
Please do not do this. I am in the process of making a major refactor of libnczarr. |
This fixes about 100 out of 130 warnings from
libnczarr. The remaining 30-ish warnings are basically either:size_twould give UBOne thing that gives rise to a bunch of warnings is
maxstrlen. It's declared as anintinNCZ_VAR_INFO_T, but used assize_tin several places. I couldn't work out if this is publicly accessible or not, or if setting it to a negative value had some semantic meaning. I think it would be safe to change tosize_tconsistently.