[01/16] checkpatch: Add xendevicemodel_handle to the list of types

This avoids checkpatch misparsing (as statements) long function
definitions or declarations, which sometimes start with constructs
like this:

  static inline int xendevicemodel_relocate_memory(
      xendevicemodel_handle *dmod, domid_t domid, ...

The type xendevicemodel_handle does not conform to Qemu CODING_STYLE,
which would suggest CamelCase.  However, it is a type defined by the
Xen Project in xen.git.  It would be possible to introduce a typedef
to allow the qemu code to refer to it by a differently-spelled name,
but that would obfuscate more than it would clarify.

CC: Eric Blake <eblake@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
v7: Added comment to commit message about why we are not renaming
    this type to CamelCase.
v6.1: New patch
---
 scripts/checkpatch.pl | 1 +
 1 file changed, 1 insertion(+)

(expanding the CC to include everyone that get_maintainer suggests)

Ian Jackson writes ("[PATCH 01/16] checkpatch: Add xendevicemodel_handle to the list of types"):
> This avoids checkpatch misparsing (as statements) long function
> definitions or declarations, which sometimes start with constructs
> like this:
> 
>   static inline int xendevicemodel_relocate_memory(
>       xendevicemodel_handle *dmod, domid_t domid, ...
> 
> The type xendevicemodel_handle does not conform to Qemu CODING_STYLE,
> which would suggest CamelCase.  However, it is a type defined by the
> Xen Project in xen.git.  It would be possible to introduce a typedef
> to allow the qemu code to refer to it by a differently-spelled name,
> but that would obfuscate more than it would clarify.

This patch has been posted in substantially similar form quite a few
times now.  Paolo Bonzini understandably suggested that renaming the
variable would be better but that's not within qemu's bailiwick as I
say above.

I think everything else in this series has a review and/or an ack.
So I would like to send a pull request.

Does someone want to review this patch ?  Should I drop it and just
let checkpatch complain ?  Shold I include it in my pull request
anyway ?

Thanks,
Ian.

On 04/26/2018 06:06 AM, Ian Jackson wrote:
> (expanding the CC to include everyone that get_maintainer suggests)
> 
> Ian Jackson writes ("[PATCH 01/16] checkpatch: Add xendevicemodel_handle to the list of types"):
>> This avoids checkpatch misparsing (as statements) long function
>> definitions or declarations, which sometimes start with constructs
>> like this:
>>
>>   static inline int xendevicemodel_relocate_memory(
>>       xendevicemodel_handle *dmod, domid_t domid, ...
>>
>> The type xendevicemodel_handle does not conform to Qemu CODING_STYLE,
>> which would suggest CamelCase.  However, it is a type defined by the
>> Xen Project in xen.git.  It would be possible to introduce a typedef
>> to allow the qemu code to refer to it by a differently-spelled name,
>> but that would obfuscate more than it would clarify.
> 
> This patch has been posted in substantially similar form quite a few
> times now.  Paolo Bonzini understandably suggested that renaming the
> variable would be better but that's not within qemu's bailiwick as I
> say above.
> 
> I think everything else in this series has a review and/or an ack.
> So I would like to send a pull request.
> 
> Does someone want to review this patch ?  Should I drop it and just
> let checkpatch complain ?  Shold I include it in my pull request
> anyway ?

If no one has commented on what seems pretty trivial (especially since
checkpatch.pl has no official maintainer, but is more of a
"whoever-touched-it-last" file at the moment), then including the patch
in your pull request is perfectly acceptable.  As a maintainer, it is
also perfectly acceptable for you to ignore false positives from
checkpatch (although documenting it in the commit message and/or cover
letter never hurts, when you are intentionally ignoring a false positive).

But, as there has also been a recent patch to teach checkpatch about
glib types [1], your patch makes sense (any merge conflict between your
patch and that one will be obvious to resolve).  So on that grounds,

Reviewed-by: Eric Blake <eblake@redhat.com>

[1] https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg04179.html

> If no one has commented on what seems pretty trivial (especially since
> checkpatch.pl has no official maintainer, but is more of a
> "whoever-touched-it-last" file at the moment), then including the patch
> in your pull request is perfectly acceptable.  As a maintainer, it is
> also perfectly acceptable for you to ignore false positives from
> checkpatch (although documenting it in the commit message and/or cover
> letter never hurts, when you are intentionally ignoring a false positive).

Thanks for the comprehensive response.

> But, as there has also been a recent patch to teach checkpatch about
> glib types [1], your patch makes sense (any merge conflict between your
> patch and that one will be obvious to resolve).  So on that grounds,
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks.

I will rebase onto current master before sending a pull request, so
I'll get to resolve that conflict.

Regards,
Ian.