Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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 more C warnings #32

Closed
schwehr opened this issue Dec 23, 2022 · 5 comments
Closed

Fix more C warnings #32

schwehr opened this issue Dec 23, 2022 · 5 comments
Assignees
Labels
Component - Build CMake, Autotools Component - C Library Core C library issues Component - netCDF netCDF interface and nc* command-line tools Component - Testing Test code, GitHub workflows Component - Tools Command-line tools like dumper and hdiff Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Improvement Improvements that don't add a new feature or functionality

Comments

@schwehr
Copy link
Collaborator

schwehr commented Dec 23, 2022

There are still a lot of folks using hdf4 around (I'm one of them). I've seen a few fuzzer bugs in prior versions and am having trouble getting all the platforms I have to support working with the version at head (e.g. x86-64, ppc, and arm). Before trying to dig into more, I figured I'd start with my current default compiler and see if I can do some low risk code cleanup if folks are up for it. My initial run as seeing what compiler warnings are showing up:

rm -rf build-ninja; time (mkdir -p build-ninja && cd build-ninja && cmake -DCMAKE_ANSI_CFLAGS:STRING="-Wall -Wextra -Werror -Wno-implicit-fallthrough -Wno-address -Wno-sign-compare -Wno-pedantic -Wno-stringop-truncation -Wno-type-limits -Wno-use-after-free -Wno-pointer-to-int-cast -Wno-pedantic -Wno-strict-aliasing -Wno-parentheses -Wno-tautological-compare -Wno-unused-parameter -Wno-strict-aliasing -Wno-discarded-qualifiers -Wno-implicit-function-declaration -Wno-alloc-size-larger-than -Wno-int-to-pointer-cast -Wno-array-bounds -Wno-unused-function -Wno-no-unused-const-variable= -Wno-unused-variable -Wno-memset-elt-size -Wno-maybe-uninitialized -Wno-switch -Wno-format-overflow -Wno-unused-but-set-variable" -GNinja .. && cmake --build . && ctest -V .)

I had trouble with being able to turn off -pedantic, so I commented that out in

set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pedantic -Wall -Wextra")

And here here in

set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pedantic -Wall -Wextra")

I will try to make some manageable pull requests that just address one of those compiler warnings at a time (as I have time).

Also, I see reference to things like VAX and $Id that are easy cleanups.

schwehr added a commit to schwehr/hdf4 that referenced this issue Dec 23, 2022
@schwehr
Copy link
Collaborator Author

schwehr commented Dec 26, 2022

I see a number of ways to quickly improve the HDF4 codebase so that it can live as long as possible with the minimum of future upkeep work. A lot of this stuff was done to GDAL and some to shapelib, mb-system, etc. e.g. OSGeo/shapelib#17 And I see that there has been some start to a few of these

  • codespell - fix the many hundreds of spelling mistakes
  • clang-format
  • Remove unused variables
  • Include what you use (iwyu)
  • Order includes
  • Remove comments that are no longer relevant
  • Remove version history messages from file comments
  • Localize variables
  • Combine definition and initialization
  • Add const when possible
  • int boolean → bool (using stdbool.h)
  • stdint.h types
  • Reorder functions to not need local prototypes
  • K&R prototypes to ANSI C prototypes (e.g. https://invisible-island.net/cproto/cproto.html and the now dead protoize) Done in protoize most functions #55
  • Remove much of the code behind various debug macros. If needed, we can make some additional unit tests
    • AN_DEBUG CDEBUG CHK_DEBUG CHK_DEBUG_1 CHK_DEBUG_10 CHK_DEBUG_2 CHK_DEBUG_3 CHK_DEBUG_4 CHK_DEBUG_5 DEBUG DEBUGGING DEBUG_HDF DISKBLOCK_DEBUG EDEBUG MALDEBUG MCACHE_DEBUG MDEBUG RDEBUG SDDEBUG SYNCDEBUG VDEBUG XDRDEBUG
  • Remove most of these #ifdef blocks:
    • added_by_mistake, DONT_KNOW, FASTER_BUT_DOESNT_WORK, FOO, LATER, MALLOC_CHECK, NOT_IMPLEMENTED, NOTNEEDED, NOTNOW, NOT_USED, NOTUSED, NOT_WORKING, NOT_YET, not_yet_implemented, OLD_WAY, TEST*, TURBOC
  • Stop testing against FALSE and TRUE
  • Remove unnecessary goto done
  • and ?

These can be a lot smaller:

find . -name "*.[chf]" | xargs wc -l | grep total
 273734 total

@schwehr
Copy link
Collaborator Author

schwehr commented Dec 29, 2022

After looking at the build system and CI setup, it looks like hdf4 would benefit from:

derobins added a commit that referenced this issue Jan 6, 2023
@schwehr
Copy link
Collaborator Author

schwehr commented Jan 10, 2023

More cleanup ideas

  • Remove all uses of register. They generally not needed with modern compilers. Done in Remove most instances of register #77
  • Remove (void) in front of printf and memset. Remove inside places like free calls that take a void *

@derobins
Copy link
Member

We'll be making many of these changes over the next year or so in an effort to make HDF4 into a more maintainable piece of software. Some of them will make it into 4.2.16, but others will have to wait until later in the year, as we get 4.2.17 together.

@schwehr
Copy link
Collaborator Author

schwehr commented Jan 30, 2023

Just a note that I've seen massive amounts of cleanup in the last couple weeks. There is alway always more that can be done, but the code is a lot easier to work with. One of the biggest improvements so far is removing a lot of the dead code.

@derobins derobins self-assigned this May 3, 2023
@derobins derobins added Component - Build CMake, Autotools Component - C Library Core C library issues Component - Tools Command-line tools like dumper and hdiff Component - netCDF netCDF interface and nc* command-line tools Component - XDR Component - Testing Test code, GitHub workflows Type - Improvement Improvements that don't add a new feature or functionality Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. labels May 3, 2023
@derobins derobins moved this to Todo in HDF4 4.3.0 May 3, 2023
@HDFGroup HDFGroup locked and limited conversation to collaborators May 8, 2023
@derobins derobins converted this issue into discussion #352 May 8, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in HDF4 4.3.0 May 8, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Component - Build CMake, Autotools Component - C Library Core C library issues Component - netCDF netCDF interface and nc* command-line tools Component - Testing Test code, GitHub workflows Component - Tools Command-line tools like dumper and hdiff Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Improvement Improvements that don't add a new feature or functionality
Projects
Status: Done
Development

No branches or pull requests

2 participants