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

Replaced ancient K&R function declarations to be C23 compatible #2801

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

seanm
Copy link
Contributor

@seanm seanm commented Nov 15, 2023

No description provided.

@seanm
Copy link
Contributor Author

seanm commented Nov 15, 2023

@WardF @DennisHeimbigner The codebase has a lot of old K&R function declarations. I thought it would just be just a few, so I started fixing them, but I soon realized there were many. So I stopped to share what I started, so it can be discussed. Is it just historical cruft that there are so many?

This syntax is no longer compiles with C23.

@DennisHeimbigner
Copy link
Collaborator

The code where this is occurring is indeed very old; from the first incarnations of the netcdf-c library.

@seanm
Copy link
Contributor Author

seanm commented Nov 15, 2023

@DennisHeimbigner ok so I can proceed to fixing them all?

Assuming so, one question: where do you prefer I move comments related to a specific variable, like this case for varid:

void
del_att (test, varid, iatt)	/* delete attribute iatt in the netcdf test */
     struct netcdf *test;
     int varid;			/* variable id */
     struct cdfatt *iatt;

should it end up like this maybe?

/* delete attribute iatt in the netcdf test */
void
del_att (struct netcdf *test,
         int varid, /* variable id */
         struct cdfatt *iatt)

@seanm seanm marked this pull request as draft November 15, 2023 23:06
@seanm
Copy link
Contributor Author

seanm commented Dec 5, 2023

@DennisHeimbigner @WardF any preference about those comment placements?

@seanm seanm force-pushed the c23-fixes branch 2 times, most recently from 7c9a15d to 3dbc0da Compare December 11, 2023 22:51
@DWesl
Copy link
Contributor

DWesl commented Dec 13, 2023

Another option, if you don't mind setting up doxygen, would be:

/** delete attribute iatt in the netcdf test
 *
 * \param varid variable id
 */
void
del_att (struct netcdf *test, int varid, att *iatt)

It looks like adding \private at the end will leave the function out of the public documentation.
Alternately,

/** delete attribute iatt in the netcdf test */
void
del_att(struct netcdf *test,
            int varid /**< variable id */,
            att *iatt)

combines doxygen with the second style above

@seanm seanm force-pushed the c23-fixes branch 2 times, most recently from 6b13eb4 to f66ca06 Compare December 13, 2023 18:52
@seanm
Copy link
Contributor Author

seanm commented Dec 13, 2023

@DWesl almost all the affected files are test files... not sure using doxygen makes sense there...?

WardF
WardF previously approved these changes Dec 13, 2023
@WardF
Copy link
Member

WardF commented Dec 13, 2023

No particular preference; Doxygen would be great (even in test files), but the lack of doxygen wouldn't prevent adoption of a PR. Thanks! We're focused on working through the backlog, and making good progress.

@seanm
Copy link
Contributor Author

seanm commented Dec 14, 2023

No particular preference

OK, this is done then IMHO.

@WardF WardF merged commit 01eae4f into Unidata:main Dec 19, 2023
99 checks passed
@edwardhartnett
Copy link
Contributor

Thanks for this!

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.

5 participants