mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Alex Williamson <alex.williamson@redhat.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Cornelia Huck <cohuck@redhat.com>,
	Laura Abbott <labbott@redhat.com>,
	Alexey Kardashevskiy <aik@ozlabs.ru>,
	kvm@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] vfio_pci: Add local source directory as include
Date: Tue, 08 Jan 2019 19:02:23 +1100	[thread overview]
Message-ID: <87pnt7tw4w.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: <20190107131341.00581863@x1.home>

Alex Williamson <alex.williamson@redhat.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

  parent reply	other threads:[~2019-01-08  8:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-04 19:57 Laura Abbott
2019-01-07  7:26 ` Alexey Kardashevskiy
2019-01-07  8:58 ` Michael Ellerman
2019-01-07 10:12   ` Masahiro Yamada
2019-01-07 11:07     ` Cornelia Huck
2019-01-07 11:39       ` Masahiro Yamada
2019-01-07 20:13         ` Alex Williamson
2019-01-07 23:52           ` Alexey Kardashevskiy
2019-01-08  0:24             ` Alex Williamson
2019-01-08  2:20               ` Alexey Kardashevskiy
2019-01-08  2:38                 ` Masahiro Yamada
2019-01-08  2:57                   ` Alexey Kardashevskiy
2019-01-08  3:19                     ` Masahiro Yamada
2019-01-08  8:02           ` Michael Ellerman [this message]
2019-01-07 20:06   ` Laura Abbott
2019-01-07 11:12 ` Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pnt7tw4w.fsf@concordia.ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=aik@ozlabs.ru \
    --cc=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=labbott@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome