Fix warnings in NCZarr tests

[ZedThree]

Fixes #2768

Fixes the remaining 80 warnings in the NCZarr tests that aren't fixed by my other PRs.

I noticed that there's a Odometer struct along with some related free functions that are duplicated with some minor variations between various tests here and in nc4_tests -- potential for consolidating those into a single test helper library and reducing duplication?

[DennisHeimbigner]

config.h should always be first, so this:

#include <stddef.h>
#ifdef HAVE_CONFIG_H
#include "config.h"
#endif

should probably be this:

#ifdef HAVE_CONFIG_H
#include "config.h"
#endif
#ifdef HAVE_STDDEF_H
#include <stddef.h>
#endif

[ZedThree]

Sure, I can make sure the config.h header is included first. The guard for stddef.h isn't really needed though, as it's a mandatory part of the C89 standard.

[DennisHeimbigner]

In some cases, converting from size_t to int is a problem
because on a 64 bit machine, sizeof(int)=4 and sizeof(size_t)=8.
For things that contain array indices (e.g. odometer) this can cause problems
because array indices can be larger than 2147483647. You would need to use
"long" instead of int.

[ZedThree]

True, blindly changing size_t to int everywhere is probably not a good idea. Here, I've only changed places which currently generate warnings from implicit type conversions because either the sign could change or the value could change (larger type to small type, for instance), so if there was an issue, we should have seen it already (or the tests need expanding to catch it!)

If I've changed the type of a function argument or a struct member, it's because I can see everywhere it's used and what type is being passed.

So, for instance I've changed Odometer::rank from size_t to int because it's always used as an int (e.g. being passed to printvector), and always constructed from an int (e.g. from int Format::rank).

I do think it's really important to try and quash these warnings, as they can hide real issues and drown out static analysis. There's still almost 2000 warnings of this kind in main, and that's after I've fixed 500 of them. Just printing that many warnings measurably slows down compilation! I would really like to get to the point where this project could enable -Werror in CI. A lot of science depends on netCDF, I'd like it to be the best it could be!

[DennisHeimbigner]

I do think it's really important to try and quash these warnings,

Agreed. And it is probable that the main-line tests do not test constrained access
to very large datasets, hence odometer values > int max are not tested.
There is an option for running large tests that is normally not enabled.
It may test large datasets, I have not checked.
In any case, Odometer must be size_t not int. There are other possible instances of this
so you must be careful to analyze the domain of the size_t object.
Finding those places where the odometer is set with an int is important and those
places need to be fixed.

[ZedThree]

Not sure why just one test is failing, looks like it failed to build, but the output just says Error: The operation was canceled and I don't see where it's actually failing.

And it is probable that the main-line tests do not test constrained access
to very large datasets, hence odometer values > int max are not tested.

Just to be clear, this has only changed Odometer::rank and not Odometer::index, and rank only needs to be large enough to store values up to NC_MAX_VAR_DIMS == 1024

[WardF]

I think the failure was just one of those things that happens sometimes with Github Actions. I re-ran the test and it appears to have succeeded.