search

shadow-maint / shadow

Upstream shadow tree
Other
292 stars 228 forks source link

lib/, src/: Rename unused macro as MAYBE_UNUSED #919

Closed psaavedra closed 7 months ago

psaavedra commented 7 months ago

Also fixes #918 by adding the omitted parameter name in active_sessions_count.

Suggested-by: Alejandro Colomar alx@kernel.org

psaavedra commented 7 months ago

Just for reference, this is the backported commit to the 2.14.x branch: https://github.com/psaavedra/shadow/commit/e1e95951b0b9553e801b57ccd153c42baa956caa. Already adapted to the differences between the branches.

alejandro-colomar commented 7 months ago

Please split the commit into two separate commits. One for renaming the macro, and one for naming the parameter.

psaavedra commented 7 months ago

Please split the commit into two separate commits. One for renaming the macro, and one for naming the parameter.

Yes, makes much more sense. Done.

alejandro-colomar commented 7 months ago

I've been thinking... Would you mind renaming the macro to MAYBE_UNUSED instead of just UNUSED? That's the name that the ISO C23 attribute has (and it is more precise), and UNUSED() is a macro name that's sometimes used for something different: discarding the return value of an expression.

alejandro-colomar commented 7 months ago

And I also see something inconsistent (not your fault): sometimes we say int unused(x) and others we say unused int x. I'd like to use the second form always. Would you mind writing a separate patch that makes the use of this macro consistent? If you do this, please put that commit as the first one if you can.

Thanks!

psaavedra commented 7 months ago

PR updated with the suggestions from @alejandro-colomar.

alejandro-colomar commented 7 months ago

Thanks!

Reviewed-by: Alejandro Colomar <alx@kernel.org>