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.

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

Fix conversion warnings in libdispatch #2905

Merged
merged 2 commits into from
Apr 22, 2024

Conversation

ZedThree
Copy link
Contributor

@ZedThree ZedThree commented Apr 8, 2024

With this PR, there will be just under 400 warnings left, down from 2400 in November. Here's a summary of what's left:

Warnings Directory
261 ncgen (see #2897)
119 libnczarr (see #2852)
75 libdap2
63 ncgen3 (see #2900)
31 libdap4 (see #2898)
27 include
23 libhdf5
13 libsrc
9 ncdump
6 build/libsrc
1 libdispatch (see below)
1 nc_test
1 oc2
386 Total

There's one remaining warning in libdispatch, and while it's an easy fix, it looks pretty suspicious:

netcdf-c/libdispatch/daux.c: In function ‘computefieldinfo’:
netcdf-c/libdispatch/daux.c:291:37: warning: conversion to ‘size_t’ {aka ‘long unsigned int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
  291 |         offset += getpadding(offset,alignment);
      |                                     ^~~~~~~~~

and here's the context:

    for(offset=0,i=0;i<cmpd->nfields;i++) {
        struct NCAUX_FIELD* field = &cmpd->fields[i];
	int alignment = 0;
	nc_type firsttype = findfirstfield(cmpd->ncid,field->fieldtype);

        /* only support 'c' alignment for now*/
	switch (field->fieldtype) {
	case NC_OPAQUE:
	    field->alignment = 1;
	    break;
	case NC_ENUM:
	    status = ncaux_type_alignment(firsttype,cmpd->ncid,&field->alignment);
	    break;
	case NC_VLEN: /*fall thru*/
	case NC_COMPOUND:
            status = ncaux_type_alignment(firsttype,cmpd->ncid,&field->alignment);
	    break;
	default:
            status = ncaux_type_alignment(field->fieldtype,cmpd->ncid,&field->alignment);
	    break;

	}
        offset += getpadding(offset,alignment);
        field->offset = offset;
        offset += field->size;
    }

Notice how alignment is initialised to zero and then never changed? This means that getpadding(...) always returns zero. Should this actually be offset += getpadding(offset, field->alignment)?

@WardF
Copy link
Member

WardF commented Apr 17, 2024

Merged the change with the Github-hosted Windows Runner test; it should be a good way to test the waters, and will also lay the groundwork for getting this in so that we can get 4.7.3 out.

@WardF WardF merged commit fc02ef4 into Unidata:main Apr 22, 2024
105 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants