On Wed, Oct 25, 2017 at 08:01:24AM +0200, Ingo Molnar wrote:
Beyond the #ifdef reduction I mentioned in the other thread, there's four other
things I noticed that need to be fixed in this patch:
- Please write out 'minor' instead of the 'm' abbreviation that is meaningless.
'm' is only used for trivial wrappers, but this wrapper is not trivial - so
proper canonical variable names should be used.
- Since __key and __name is already double underscores that is customary for
macros to avoid variable name shadowing, why is 'ret' not such a name?
- But, 'ret' is the typical name used for integer returns, not for pointers!
Please check the gendisk code for what the typical name for gendisk pointers
is and use that instead of making up new, weird patterns ...
- The "(complete)"#minor"("#id")" generated name is pretty bad. Firstly
"complete" is a verb (or adjective), while lock(dep) symbol names should be
nouns! But even "completion" is pretty opaque, how about "gendisk_completion"?
More careful patches please!
I am sorry for that. I will do my best not to repeat them.
And I re-spined ASAP to make you able to review with the latest version
only including all your suggestions at the previous version.