Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix many warnings in libnczarr #2852

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix many warnings in libnczarr #2852

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
51073cb
Silence some conversion warnings in libnczarr
ZedThree Jan 15, 2024
013af12
Change `len` member of `NC_ATT_INFO_T` to `size_t`
ZedThree Jan 16, 2024
585e322
Switch some `int/mode_t` confusion
ZedThree Jan 31, 2024
3537781
Fix some sign conversion warnings in libnczarr functions/types
ZedThree Jan 31, 2024
f8da95a
Fix loop conditional in `NCZ_filter_finalize`
ZedThree Mar 12, 2024
0df2958
Merge branch 'main' into silence-libnczarr-warnings
ZedThree Mar 25, 2024
dac5355
Conditionally mark variable as unused
ZedThree Mar 25, 2024
e7c1c78
Silence remaining nczarr conversion warnings
ZedThree Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/nc4internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ typedef struct NC_ATT_INFO
{
NC_OBJ hdr; /**< The hdr contains the name and ID. */
struct NC_OBJ *container; /**< Pointer to containing group|var. */
int len; /**< Length of attribute data. */
size_t len; /**< Length of attribute data. */
nc_bool_t dirty; /**< True if attribute modified. */
nc_bool_t created; /**< True if attribute already created. */
nc_type nc_typeid; /**< NetCDF type of attribute's data. */
Expand Down
2 changes: 1 addition & 1 deletion libhdf5/hdf5attr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ nc4_put_att(NC_GRP_INFO_T* grp, int varid, const char *name, nc_type file_type,
/* For an existing att, if we're not in define mode, the len
must not be greater than the existing len for classic model. */
if (!(h5->flags & NC_INDEF) &&
len * nc4typelen(file_type) > (size_t)att->len * nc4typelen(att->nc_typeid))
len * nc4typelen(file_type) > att->len * nc4typelen(att->nc_typeid))
{
if (h5->cmode & NC_CLASSIC_MODEL)
return NC_ENOTINDEFINE;
Expand Down
5 changes: 3 additions & 2 deletions libnczarr/zarr.c
View file
Open in desktop
Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner Jan 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. ncconfigure.h is normally included by including config.h.
    config.h should be in zincludes.h
  2. zinfo->controls.flags &= (size64_t)(~noflags);
    should this be zinfo->controls.flags &= ~((size64_t)(noflags));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Ah sorry, my editor automatically includes the most relevant header when inserting new types. It looks like nothing in zincludes.h is used directly which is probably why it chose ncconfigure.h instead.

  2. It's probably actually clearer to change the type of noflags itself, not sure why I didn't spot that, thanks

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* See netcdf/COPYRIGHT file for copying and redistribution conditions.
*********************************************************************/

#include "ncconfigure.h"
#include "zincludes.h"
#include <stddef.h>

Expand Down Expand Up @@ -232,7 +233,7 @@ int
NCZ_get_superblock(NC_FILE_INFO_T* file, int* superblockp)
{
NCZ_FILE_INFO_T* zinfo = file->format_file_info;
if(superblockp) *superblockp = zinfo->zarr.nczarr_version.major;
if(superblockp) *superblockp = (int)zinfo->zarr.nczarr_version.major;
return NC_NOERR;
}

Expand Down Expand Up @@ -337,7 +338,7 @@ applycontrols(NCZ_FILE_INFO_T* zinfo)
}
/* Apply negative controls by turning off negative flags */
/* This is necessary to avoid order dependence of mode flags when both positive and negative flags are defined */
zinfo->controls.flags &= (~noflags);
zinfo->controls.flags &= (size64_t)(~noflags);

/* Process other controls */
if((value = controllookup(zinfo->controllist,"log")) != NULL) {
Expand Down
8 changes: 7 additions & 1 deletion libnczarr/zarr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
#ifndef ZARR_H
#define ZARR_H

#include "nc4internal.h"
#include "ncbytes.h"
#include "ncjson.h"
#include "netcdf.h"
#include "zinternal.h"

struct ChunkKey;
struct S3credentials;

Expand Down Expand Up @@ -78,7 +84,7 @@ EXTERNL int NCZ_subobjects(NCZMAP* map, const char* prefix, const char* tag, cha
EXTERNL int NCZ_grpname_full(int gid, char** pathp);
EXTERNL int ncz_get_var_meta(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var);
EXTERNL int NCZ_comma_parse(const char* s, NClist* list);
EXTERNL int NCZ_swapatomicdata(size_t datalen, void* data, int typesize);
EXTERNL int NCZ_swapatomicdata(size_t datalen, void* data, size_t typesize);
EXTERNL char** NCZ_clonestringvec(size_t len, const char** vec);
EXTERNL void NCZ_freestringvec(size_t len, char** vec);
EXTERNL int NCZ_ischunkname(const char* name,char dimsep);
Expand Down
21 changes: 9 additions & 12 deletions libnczarr/zattr.c
View file
Open in desktop
Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner Jan 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ ncz_getattlist(NC_GRP_INFO_T *grp, int varid, NC_VAR_INFO_T **varp, NCindex **at
{
NC_VAR_INFO_T *var;

if (!(var = (NC_VAR_INFO_T *)ncindexith(grp->vars, varid)))
if (!(var = (NC_VAR_INFO_T *)ncindexith(grp->vars, (size_t)varid)))
return NC_ENOTVAR;
assert(var->hdr.id == varid);

Expand Down Expand Up @@ -120,14 +120,13 @@ ncz_get_att_special(NC_FILE_INFO_T* h5, NC_VAR_INFO_T* var, const char* name,

/* The global reserved attributes */
if(strcmp(name,NCPROPS)==0) {
int len;
if(h5->provenance.ncproperties == NULL)
{stat = NC_ENOTATT; goto done;}
if(mem_type == NC_NAT) mem_type = NC_CHAR;
if(mem_type != NC_CHAR)
{stat = NC_ECHAR; goto done;}
if(filetypep) *filetypep = NC_CHAR;
len = strlen(h5->provenance.ncproperties);
size_t len = strlen(h5->provenance.ncproperties);
if(lenp) *lenp = len;
if(data) strncpy((char*)data,h5->provenance.ncproperties,len+1);
} else if(strcmp(name,ISNETCDF4ATT)==0
Expand All @@ -138,7 +137,7 @@ ncz_get_att_special(NC_FILE_INFO_T* h5, NC_VAR_INFO_T* var, const char* name,
if(strcmp(name,SUPERBLOCKATT)==0)
iv = (unsigned long long)h5->provenance.superblockversion;
else /* strcmp(name,ISNETCDF4ATT)==0 */
iv = NCZ_isnetcdf4(h5);
iv = (unsigned long long)NCZ_isnetcdf4(h5);
if(mem_type == NC_NAT) mem_type = NC_INT;
if(data)
switch (mem_type) {
Expand Down Expand Up @@ -279,8 +278,7 @@ NCZ_del_att(int ncid, int varid, const char *name)
NC_FILE_INFO_T *h5;
NC_ATT_INFO_T *att;
NCindex* attlist = NULL;
int i;
size_t deletedid;
int deletedid;
int retval;

/* Name must be provided. */
Expand Down Expand Up @@ -354,7 +352,7 @@ NCZ_del_att(int ncid, int varid, const char *name)
return retval;

/* Renumber all attributes with higher indices. */
for (i = 0; i < ncindexsize(attlist); i++)
for (size_t i = 0; i < ncindexsize(attlist); i++)
{
NC_ATT_INFO_T *a;
if (!(a = (NC_ATT_INFO_T *)ncindexith(attlist, i)))
Expand All @@ -379,7 +377,7 @@ NCZ_del_att(int ncid, int varid, const char *name)
* @return Type size in bytes, or -1 if type not found.
* @author Dennis Heimbigner, Ed Hartnett
*/
static int
static size_t
nc4typelen(nc_type type)
{
switch(type){
Expand All @@ -399,7 +397,7 @@ nc4typelen(nc_type type)
case NC_UINT64:
return 8;
}
return -1;
return 0;
}

/**
Expand Down Expand Up @@ -516,7 +514,7 @@ ncz_put_att(NC_GRP_INFO_T* grp, int varid, const char *name, nc_type file_type,
/* For an existing att, if we're not in define mode, the len
must not be greater than the existing len for classic model. */
if (!(h5->flags & NC_INDEF) &&
len * nc4typelen(file_type) > (size_t)att->len * nc4typelen(att->nc_typeid))
len * nc4typelen(file_type) > att->len * nc4typelen(att->nc_typeid))
{
if (h5->cmode & NC_CLASSIC_MODEL)
return NC_ENOTINDEFINE;
Expand Down Expand Up @@ -980,7 +978,6 @@ int
ncz_create_fillvalue(NC_VAR_INFO_T* var)
{
int stat = NC_NOERR;
int i;
NC_ATT_INFO_T* fv = NULL;

/* Have the var's attributes been read? */
Expand All @@ -989,7 +986,7 @@ ncz_create_fillvalue(NC_VAR_INFO_T* var)
/* Is FillValue warranted? */
if(!var->no_fill && var->fill_value != NULL) {
/* Make sure _FillValue does not exist */
for(i=0;i<ncindexsize(var->att);i++) {
for(size_t i=0;i<ncindexsize(var->att);i++) {
fv = (NC_ATT_INFO_T*)ncindexith(var->att,i);
if(strcmp(fv->hdr.name,_FillValue)==0) break;
fv = NULL;
Expand Down
10 changes: 4 additions & 6 deletions libnczarr/zchunking.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,8 @@ NCZ_compute_chunk_ranges(
NCZChunkRange* ncr)
{
int stat = NC_NOERR;
int i;
int rank = common->rank;

for(i=0;i<rank;i++) {
for(size_t i=0;i<common->rank;i++) {
if((stat = compute_intersection(&slices[i],common->chunklens[i],common->isunlimited[i],&ncr[i])))
goto done;
}
Expand Down Expand Up @@ -84,7 +82,7 @@ This is somewhat complex because:
*/

int
NCZ_compute_projections(struct Common* common, int r, size64_t chunkindex, const NCZSlice* slice, size_t n, NCZProjection* projections)
NCZ_compute_projections(struct Common* common, size_t r, size64_t chunkindex, const NCZSlice* slice, size_t n, NCZProjection* projections)
{
int stat = NC_NOERR;
NCZProjection* projection = NULL;
Expand Down Expand Up @@ -215,7 +213,7 @@ Create a vector of projections wrt a slice and a sequence of chunks.
int
NCZ_compute_per_slice_projections(
struct Common* common,
int r, /* which dimension are we projecting? */
size_t r, /* which dimension are we projecting? */
const NCZSlice* slice, /* the slice for which projections are computed */
const NCZChunkRange* range, /* range */
NCZSliceProjections* slp)
Expand Down Expand Up @@ -290,7 +288,7 @@ verifyslice(const NCZSlice* slice)
}

void
NCZ_clearsliceprojections(int count, NCZSliceProjections* slpv)
NCZ_clearsliceprojections(size_t count, NCZSliceProjections* slpv)
{
if(slpv != NULL) {
int i;
Expand Down
12 changes: 6 additions & 6 deletions libnczarr/zchunking.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ typedef struct NCProjection {

/* Set of Projections for a slice */
typedef struct NCZSliceProjections {
int r; /* 0<=r<rank */
size_t r; /* 0<=r<rank */
NCZChunkRange range; /* Chunk ranges covered by this set of projections */
size_t count; /* |projections| == (range.stop - range.start) */
NCZProjection* projections; /* Vector of projections derived from the
Expand All @@ -62,7 +62,7 @@ struct Common {
NC_VAR_INFO_T* var;
struct NCZChunkCache* cache;
int reading; /* 1=> read, 0 => write */
int rank;
size_t rank;
int scalar; /* 1 => scalar variable */
size64_t dimlens[NC_MAX_VAR_DIMS];
unsigned char isunlimited[NC_MAX_VAR_DIMS];
Expand All @@ -81,8 +81,8 @@ struct Common {
/**************************************************/
/* From zchunking.c */
EXTERNL int NCZ_compute_chunk_ranges(struct Common*, const NCZSlice*, NCZChunkRange* ncr);
EXTERNL int NCZ_compute_projections(struct Common*, int r, size64_t chunkindex, const NCZSlice* slice, size_t n, NCZProjection* projections);
EXTERNL int NCZ_compute_per_slice_projections(struct Common*, int rank, const NCZSlice*, const NCZChunkRange*, NCZSliceProjections* slp);
EXTERNL int NCZ_compute_projections(struct Common*, size_t r, size64_t chunkindex, const NCZSlice* slice, size_t n, NCZProjection* projections);
EXTERNL int NCZ_compute_per_slice_projections(struct Common*, size_t rank, const NCZSlice*, const NCZChunkRange*, NCZSliceProjections* slp);
EXTERNL int NCZ_compute_all_slice_projections(struct Common*, const NCZSlice* slices, const NCZChunkRange*, NCZSliceProjections*);

/* From zwalk.c */
Expand All @@ -98,8 +98,8 @@ EXTERNL size64_t NCZ_computelinearoffset(size_t, const size64_t*, const size64_t
struct Common;
struct NCZOdometer;
EXTERNL int NCZ_projectslices(struct Common*, NCZSlice* slices, struct NCZOdometer**);
EXTERNL int NCZ_chunkindexodom(int rank, const NCZChunkRange* ranges, size64_t*, struct NCZOdometer** odom);
EXTERNL void NCZ_clearsliceprojections(int count, NCZSliceProjections* slpv);
EXTERNL int NCZ_chunkindexodom(size_t rank, const NCZChunkRange* ranges, size64_t*, struct NCZOdometer** odom);
EXTERNL void NCZ_clearsliceprojections(size_t count, NCZSliceProjections* slpv);
EXTERNL void NCZ_clearcommon(struct Common* common);

#define floordiv(x,y) ((x) / (y))
Expand Down
20 changes: 7 additions & 13 deletions libnczarr/zclose.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,13 @@ zclose_group(NC_GRP_INFO_T *grp)
{
int stat = NC_NOERR;
NCZ_GRP_INFO_T* zgrp;
int i;

assert(grp && grp->format_grp_info != NULL);
LOG((3, "%s: grp->name %s", __func__, grp->hdr.name));

/* Recursively call this function for each child, if any, stopping
* if there is an error. */
for(i=0; i<ncindexsize(grp->children); i++) {
for(size_t i=0; i<ncindexsize(grp->children); i++) {
if ((stat = zclose_group((NC_GRP_INFO_T*)ncindexith(grp->children,i))))
goto done;
}
Expand Down Expand Up @@ -123,8 +122,7 @@ zclose_gatts(NC_GRP_INFO_T* grp)
{
int stat = NC_NOERR;
NC_ATT_INFO_T *att;
int a;
for(a = 0; a < ncindexsize(grp->att); a++) {
for(size_t a = 0; a < ncindexsize(grp->att); a++) {
NCZ_ATT_INFO_T* zatt = NULL;
att = (NC_ATT_INFO_T* )ncindexith(grp->att, a);
assert(att && att->format_att_info != NULL);
Expand All @@ -149,11 +147,10 @@ NCZ_zclose_var1(NC_VAR_INFO_T* var)
int stat = NC_NOERR;
NCZ_VAR_INFO_T* zvar;
NC_ATT_INFO_T* att;
int a;

assert(var && var->format_var_info);
zvar = var->format_var_info;;
for(a = 0; a < ncindexsize(var->att); a++) {
for(size_t a = 0; a < ncindexsize(var->att); a++) {
NCZ_ATT_INFO_T* zatt;
att = (NC_ATT_INFO_T*)ncindexith(var->att, a);
assert(att && att->format_att_info);
Expand Down Expand Up @@ -191,9 +188,8 @@ zclose_vars(NC_GRP_INFO_T* grp)
{
int stat = NC_NOERR;
NC_VAR_INFO_T* var;
int i;

for(i = 0; i < ncindexsize(grp->vars); i++) {
for(size_t i = 0; i < ncindexsize(grp->vars); i++) {
var = (NC_VAR_INFO_T*)ncindexith(grp->vars, i);
assert(var && var->format_var_info);
if((stat = NCZ_zclose_var1(var))) goto done;
Expand All @@ -215,9 +211,8 @@ zclose_dims(NC_GRP_INFO_T* grp)
{
int stat = NC_NOERR;
NC_DIM_INFO_T* dim;
int i;

for(i = 0; i < ncindexsize(grp->dim); i++) {
for(size_t i = 0; i < ncindexsize(grp->dim); i++) {
NCZ_DIM_INFO_T* zdim;
dim = (NC_DIM_INFO_T*)ncindexith(grp->dim, i);
assert(dim && dim->format_dim_info);
Expand Down Expand Up @@ -265,10 +260,9 @@ static int
zclose_types(NC_GRP_INFO_T* grp)
{
int stat = NC_NOERR;
int i;
NC_TYPE_INFO_T* type;

for(i = 0; i < ncindexsize(grp->type); i++)
for(size_t i = 0; i < ncindexsize(grp->type); i++)
{
type = (NC_TYPE_INFO_T*)ncindexith(grp->type, i);
if((stat = zclose_type(type))) goto done;
Expand All @@ -289,7 +283,7 @@ static int
zwrite_vars(NC_GRP_INFO_T *grp)
{
int stat = NC_NOERR;
int i;
size_t i;

assert(grp && grp->format_grp_info != NULL);
LOG((3, "%s: grp->name %s", __func__, grp->hdr.name));
Expand Down
2 changes: 1 addition & 1 deletion libnczarr/zcvt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
Code taken directly from libdap4/d4cvt.c
*/

static const int ncz_type_size[NC_MAX_ATOMIC_TYPE+1] = {
static const size_t ncz_type_size[NC_MAX_ATOMIC_TYPE+1] = {
0, /*NC_NAT*/
sizeof(char), /*NC_BYTE*/
sizeof(char), /*NC_CHAR*/
Expand Down
Loading
Loading