Hi Eugeniu, On Monday, 4 December 2017 20:10:34 EET Eugeniu Rosca wrote: > On Mon, Dec 04, 2017 at 12:52:17PM +0200, Laurent Pinchart wrote: > > On Friday, 24 November 2017 20:40:57 EET Kieran Bingham wrote: > >> Hi Eugeniu, > >> > >> Thankyou for the patch, > >> > >> Laurent - Any comments on this? Otherwise I'll bundle this in with my > >> suspend/resume patch for a pull request. > >> > >> <CUT> > >> > >> I was going to say: We know the object is an entity by it's type. Isn't > >> hgo more descriptive for it's name ? > >> > >> However to answer my own question - The implementation function goes on > >> to define a struct vsp1_hgo *hgo, so no ... the Entity object shouldn't > >> be hgo. > > > > And that's exactly why there's a difference between the declaration and > > implementation :-) Naming the parameter hgo in the declaration makes it > > clear to the reader what entity is expected. The parameter is then named > > entity in the function definition to allow for the vsp1_hgo *hgo local > > variable. > > > > Is the mismatch a real issue ? I don't think the kernel coding style > > mandates parameter names to be identical between function declaration and > > definition. > > You are the DRM/VSP1 and kernel experts, so feel free to drop the patch. > Still IMO what makes kernel coding style sweet is its simplicity [1]. > Here is some statistics computed with exuberant ctags and cpccheck. > > $ git describe HEAD > v4.15-rc2 > > $ ctags --version > Exuberant Ctags 5.9~svn20110310, Copyright (C) 1996-2009 Darren Hiebert > Addresses: <dhiebert@xxxxxxxxxxxxxxxxxxxxx>, > http://ctags.sourceforge.net > Optional compiled features: +wildcards, +regex > > # Number of function definitions in drivers/media: > $ find drivers/media -type d | \ > xargs -I {} sh -c "ctags -x --c-types=f {}/*" | wc -l > 24913 > > # Number of func declaration/definition mismatches found by cppcheck: > $ cppcheck --force --enable=all --inconclusive drivers/media/ 2>&1 | \ > grep declaration | wc -l > 168 > > It looks like only (168/24913) * 100% = 0,67% of all drivers/media > functions have a mismatch between function declaration and function > definition. Why making this number worse? Of course the goal is not to introduce a mismatch for the sake of it. It makes sense for the declaration and definition to match in most cases. My point is that in this particular case I believe the mismatch makes the code more readable. -- Regards, Laurent Pinchart
