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 most warnings in libdap2 #2887

Merged
merged 8 commits into from
Jul 15, 2024
Merged

Conversation

ZedThree
Copy link
Contributor

I think most of the remaining warnings in libdap2 are suspicious. For example, there are some (duplicated, but that's a different matter) functions for finding the index of an element in an NClist that return -1 if the element isn't found. This is never checked, and is used as a size_t. There are various options for fixing this, but I've left them as-is, or changed types so that the warning is a much more explicit:

warning: unsigned conversion from ‘int’ to ‘size_t’ {aka ‘long unsigned int’} changes value from ‘-1’ to ‘18446744073709551615’

@ZedThree ZedThree force-pushed the silence-libdap2-warnings branch from d25d527 to 1432861 Compare March 13, 2024 15:42
Slightly odd code because this is the else-branch of a conditional
that is itself compile-time conditional
`tmpname` is allocated on the stack so can never be `NULL`. The
intention was presumably to check the return value of `strdup` instead
@ZedThree ZedThree force-pushed the silence-libdap2-warnings branch from 1432861 to fa573e7 Compare March 14, 2024 13:51
@edwardhartnett
Copy link
Contributor

Thanks for all this warnings work!

Every time we release code with warnings, an angel kicks a kitten right in the face.

@ZedThree
Copy link
Contributor Author

In my branch with everything merged, I'm down to 370 warnings from 2405 in November.

* main: (158 commits)
  Update release notes.
  Refactor macro _FillValue to NC_FillValue in support of Unidata#2858
  Remove appveyor config file.
  Add CI for a Windows Runner on Github Actions.
  Add cmake prefix path to appveyor config.
  Attempt to fix zlib-related error in appveyor.
  Correct typo.
  Remove check for libcurl unless it is necessary for required functionality.
  Rename the vendored strlcat symbol
  Fix conversion warnings in libdispatch
  Set flags to avoid warning messages if curl isn't found.
  Use modern cmake nomenclature for curl.
  Fix truncated-format warning in ncgen
  Fix some conversion warnings in ncgen3
  Fix some conversion warnings in ncgen3 generated files
  Regenerate ncgen3
  CMake: Add option to automatically regenerate ncgen3/ncgen
  Skip checking for duplicates if only one element in list
  Change format of backwards-loops
  Silence some conversion warnings in ncgen generated files
  ...
@ZedThree
Copy link
Contributor Author

There are 11 warnings remaining in libdap2 and they're all suspicious, for example:

static size_t
findin(CDFnode* parent, CDFnode* child)
{
    NClist* subnodes = parent->subnodes;
    for(size_t i=0;i<nclistlength(subnodes);i++) {
	if(nclistget(subnodes,i) == child)
	    return i;
    }
    return -1;  // Conversion warning
}

There's also no checking at the call site, so I guess this is expected to always find child? In which case, maybe it's better to hard error here?

The other warnings are similar, indicating missing checks.

ZedThree and others added 2 commits May 16, 2024 10:08
* main: (29 commits)
  ftp --> resources
  Update release notes
  Convert the ENABLE_XXX options to NETCDF_ENABLE_XXX options
  Modify CMakeLists.txt to honor CMAKE_INSTALL_MANDIR in support of Unidata#2920.
  Modify ncdump to print char-valued variables as utf8.
  CI: Create an MSYS2/MinGW CMake run.
  CI: Test on MSYS2/UCRT64 environment
  CMake: Fix running tests on MinGW
  removal of ftp and contrib site
  removal of ftp site
  removal of ftp and netcdf contrib site
  removal of ftp and netcdf contrib site
  comment
  ftp --> resources for sample data location
  ftp--> resources for sample data location
  CMake: Enable plugins on MinGW
  Reintroduce targets to allow for cmake-based netCDF-Fortran to find and linka gainst netCDF-C. Stop-gap measure until we can modify netCDF-Fortran to use a more modern approach.
  revert
  Fix conversion warning in flag set/check/clear macros
  ckp
  ...
@WardF WardF merged commit aff08d8 into Unidata:main Jul 15, 2024
107 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.

3 participants