All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.