public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: quic_zijuhu <quic_zijuhu@quicinc.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 1/5] driver core: Sort headers
Date: Thu, 22 Aug 2024 14:50:00 +0300	[thread overview]
Message-ID: <Zscl6G82YY1c-Lb3@smile.fi.intel.com> (raw)
In-Reply-To: <36bc2da3-80c2-467d-bcdd-0797809ace72@quicinc.com>

On Thu, Aug 22, 2024 at 11:30:07AM +0800, quic_zijuhu wrote:
> On 8/21/2024 11:48 PM, Andy Shevchenko wrote:
> > Sort the headers in alphabetic order in order to ease
> > the maintenance for this part.

...

> i don't think it is good idea to sort headers by alphabetic order.

I strongly disagree on this on several points:

- the header dependencies has to be resolved on each header by applying IWYU
  (Include What You Use) principle:

    in this case we don't care what is needed for each header in question

- the end developer shouldn't care about header dependencies changes as
  the project is evolving:

    it's way out of human being capacity to follow _all_ the changes in the Linux
    kernel headers

- it's much easier to maintain the inclusion block when it's sorted (to avoid
  dups, or to see in a fast manner what's already included):

    we are writing code for humans, and not for the machines (leave the
    optimisation task to the compiler in many cases)

- overall it makes the development process much easier as a whole:

    I do not believe there is a single person in the world who may tell you
    the correct order of inclusion to any, even simple, Linux kernel driver

> why ?
> 
> 1) header's dependency is not related to its file (name|path), their
> dependency are related to # includes order.

That's not true. More precisely we are working hard to make it not true (and
it's not a Plan 9 OS where as far as I know the idea was that developer knows
the exact order).

> 2) it maybe be easy to cause build error.

Yes, and again we are trying to avoid this by enforcing IWYU principle.

> 3) header's path or name maybe be related to subsystem, it is not good
> to sort one subsystem's headers before the other.

There is a grouping approach which makes this easier to get. See IIO subsystem
as a prime example for IWYU implementation in the Linux kernel.

> For header's order, my points is that:
> 
> 1) sort by their dependency.

See above. No way, it's completely impractical.

>    #include <b_header.h>
>    #include <a_header.h>
>    if
>    a_header.h:
>    #include <b_header.h>
> 
> 2) all #include <> block before all #include "" block.
> 
> 3) sort headers related to source file at the last.
> 
>    prefix_xyz.c:
> 
>    #include <>
>    .....
>    #include <prefix_xyz.h>   // it is the last if it is exposed.
> 
>    #include "internal_header.h"
>    ....
> 
> 4)
> sort relevant header together as far as possible, for example, they
> belong to the same subsystem.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2024-08-22 11:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-21 15:48 [PATCH v1 0/5] driver core: fw_devlink: Clean up strings and mutex usages Andy Shevchenko
2024-08-21 15:48 ` [PATCH v1 1/5] driver core: Sort headers Andy Shevchenko
2024-08-22  3:30   ` quic_zijuhu
2024-08-22 11:50     ` Andy Shevchenko [this message]
2024-08-21 15:48 ` [PATCH v1 2/5] driver core: Use kasprintf() instead of fixed buffer formatting Andy Shevchenko
2024-08-21 15:48 ` [PATCH v1 3/5] driver core: Use guards for simple mutex locks Andy Shevchenko
2024-08-21 15:48 ` [PATCH v1 4/5] driver core: Make use of returned value of dev_err_probe() Andy Shevchenko
2024-08-21 15:48 ` [PATCH v1 5/5] driver core: Use 2-argument strscpy() Andy Shevchenko
2024-08-22  7:39 ` [PATCH v1 0/5] driver core: fw_devlink: Clean up strings and mutex usages Greg Kroah-Hartman

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=Zscl6G82YY1c-Lb3@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quic_zijuhu@quicinc.com \
    --cc=rafael@kernel.org \
    /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