linux-kernel - Re: [PATCH] vfio_pci: Add local source directory as include

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 08 Jan 2019 19:02:23 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Alex Williamson <alex.williamson@...hat.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     Cornelia Huck <cohuck@...hat.com>,
        Laura Abbott <labbott@...hat.com>,
        Alexey Kardashevskiy <aik@...abs.ru>, kvm@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] vfio_pci: Add local source directory as include

Alex Williamson <alex.williamson@...hat.com> writes:
...
>
> Numbering options for clarity:
>
> 1)
>> ccflags-y += -I$(src)
>> would add the header search path for all files in drivers/vfio/pci/
>> whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it.
>> 
>
> 2)
>> CFLAGS_vfio_pci_nvlink2.o += -I$(src)
>> is a bit better.
>> However, it is not obvious why this extra header search path is needed
>> until you find vfio_pci_nvlink2.c including trace.h
>> 
>
> 3)
>> #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci
>> clarifies the intention because the related code is all placed in trace.h
 
Good summary.

>> From the comment in include/trace/define_trace.h
>> TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h
>
> In my scan of the tree, the most common solution seems to be 2) as this
> is essentially recommended in the sample file.  3) is well represented,
> with much fewer examples of 1), though it might depend how liberally
> we grep out or examine the use cases.

It seems to me that 1 and 2 is overwhelmingly used:

$ git grep -F "#define TRACE_INCLUDE_PATH" | wc -l
133

That counts all definitions of TRACE_INCLUDE_PATH.

$ git grep -F "#define TRACE_INCLUDE_PATH ." | wc -l
122

That's all files using '.', so only 11 locations use the relative path
method (3).


Which is unsurprising given that the sample uses '.'.

And people often look at existing code for an example, so they're also
going to tend to use '.'.


I agree with Masahiro that adding include paths to the Makefile for
this is a bit gross, and method 3 is much more preferable.

Fixing all the existing code to use method 3 would be a good beginner
project :)

cheers