NOAA-EMC / NCEPLIBS-bufr

The NCEPLIBS-bufr library contains routines and utilites for working with the WMO BUFR format.
Other
44 stars 19 forks source link

old-style function definitions used in cread.c should be converted to modern style #421

Closed edwardhartnett closed 1 year ago

edwardhartnett commented 1 year ago

In cread.c we have code like this:

/**
 * This subroutine opens a new system file for reading BUFR messages.
 *
 * @param[in] nfile -- f77int*: Internal Fortran I/O stream index associated with ufile
 * @param[in] ufile -- char*: [path/]name of system file to be opened
 *
 * @author J. Woollen @date 2012-09-15
 */
void openrb   (nfile,ufile) f77int *nfile; char *ufile; { pb[*nfile] = fopen( ufile , "rb " ); }

Note the old-style function definition.

Instead, this should be:

void 
openrb(f77int *nfile, char *ufile)
{ 
   pb[*nfile] = fopen( ufile , "rb " ); 
}

Note that the modern style of function definition is used. It has benefits over the old-style (see https://splichal.eu/scripts/sphinx/gcc/_build/html/extensions-to-the-c-language-family/prototypes-and-old-style-function-definitions.html) but even if it did not, the modern style is more compact and familiar to C programmers.

Note also that putting the entire function on one line of code takes the terseness of C beyond its usual limits. Let's keep at least one line per semicolon, as is commonly done in C code.

However this must be done carefully. Old-style function definitions cause less argument checking to be done, so there may be some cases where incorrect number or type of arguments are being passed to these functions, and changing to modern style function definitions will cause something to break.

Testing should be complete before anything beyond whitespace formatting is attempted, and also a bunch of C-only tests for these functions so that we're not just testing them indirectly, but have direct testing of everything. See https://ee.hawaii.edu/~tep/EE160/Book/chapapx/node7.html for some detail about other pitfalls to watch out for.