Am 07.06.2014 16:58, schrieb Peter Maydell: > On 6 June 2014 08:17, Markus Armbruster <arm...@redhat.com> wrote: >> --debug values=1 produces >> >> 188 > . extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid, >> 188 > EEVVVVVVVTTTTVVVVVVVVVVVVVVVVVVVVNTTTTTTTTTTTTTTVVCCTTTTTTTTVVVVVVVCC >> >> which suggests it recognizes the declaration just fine. >> >> Copying a few possible victims^Wexperts. > > It's clearly something to do with it getting confused by the type name, > because you can suppress the warning by just changing it so it has > an "_t" suffix, for instance. In particular notice that the set of allowed > type names constructed by the build_types() subroutine is extremely > limited: it looks to me as if the script makes assumptions based on > kernel style (where 'struct foo' is preferred over a typedef and plain > 'foo') that allow it to assume that if it's not one of a very few allowed > formats then it's not a type name. > > I find this script extremely hard to understand, though. > > thanks > -- PMM >
Yes, but that's only part of the story. checkpatch.pl contains some special handling for the standard C data types and also knows some Linux data type patterns. It also handles the above case correctly in most circumstances because there is special code for "*" used in function argument lists. Here checkpatch.pl gets confused by a totally different line of the patch: type_init(register_vfio_pci_dev_type) Simply adding a semicolon at the end of that line would help, but is of course not correct (note that there is already some QEMU code which adds such a semicolon and which should be fixed). It's generally very difficult or even impossible for code analysers with a limited view to analyse macro calls correctly. A human reviewer has the same problem. checkpatch.pl could be improved to handle the patch correctly: the type_init line and the line which raises the error message belong to different files. Obviously some global state variables are not reset when checkpatch.pl detects patch code belonging to a new file. I'm still searching which ones. A similar problem might also occur when more than one patch is checked: this also does not reset all relevant variables when switching from one patch file to the next one. The original Linux version of checkpatch.pl shows the same bug. Regards Stefan PS. I just sent a first patch for checkpatch.pl which is totally unrelated to the above problem, but which I noticed while I investigated it.