Sort the headers in alphabetic order in order to ease
the maintenance for this part.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 144cb8c201fb..6b9d3d255135 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -9,29 +9,29 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/blkdev.h>
 #include <linux/cpufreq.h>
 #include <linux/device.h>
+#include <linux/dma-map-ops.h> /* for dma_default_coherent */
 #include <linux/err.h>
 #include <linux/fwnode.h>
 #include <linux/init.h>
+#include <linux/kdev_t.h>
 #include <linux/kstrtox.h>
 #include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/kdev_t.h>
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
 #include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/blkdev.h>
-#include <linux/mutex.h>
 #include <linux/pm_runtime.h>
-#include <linux/netdevice.h>
 #include <linux/rcupdate.h>
-#include <linux/sched/signal.h>
 #include <linux/sched/mm.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
 #include <linux/string_helpers.h>
 #include <linux/swiotlb.h>
 #include <linux/sysfs.h>
-#include <linux/dma-map-ops.h> /* for dma_default_coherent */
 
 #include "base.h"
 #include "physical_location.h"
-- 
2.43.0.rc1.1336.g36b5255a03ac

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.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/base/core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 144cb8c201fb..6b9d3d255135 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -9,29 +9,29 @@
>   */
>  
>  #include <linux/acpi.h>
> +#include <linux/blkdev.h>
>  #include <linux/cpufreq.h>
>  #include <linux/device.h>
> +#include <linux/dma-map-ops.h> /* for dma_default_coherent */
>  #include <linux/err.h>
>  #include <linux/fwnode.h>
>  #include <linux/init.h>
> +#include <linux/kdev_t.h>
>  #include <linux/kstrtox.h>
>  #include <linux/module.h>
> -#include <linux/slab.h>
> -#include <linux/kdev_t.h>
> +#include <linux/mutex.h>
> +#include <linux/netdevice.h>
>  #include <linux/notifier.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> -#include <linux/blkdev.h>
> -#include <linux/mutex.h>
>  #include <linux/pm_runtime.h>
> -#include <linux/netdevice.h>
>  #include <linux/rcupdate.h>
> -#include <linux/sched/signal.h>
>  #include <linux/sched/mm.h>
> +#include <linux/sched/signal.h>
> +#include <linux/slab.h>
>  #include <linux/string_helpers.h>
>  #include <linux/swiotlb.h>
>  #include <linux/sysfs.h>
> -#include <linux/dma-map-ops.h> /* for dma_default_coherent */
>  
>  #include "base.h"
>  #include "physical_location.h"

Hi Andy,

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

why ?

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

2) it maybe be easy to cause build error.

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


For header's order, my points is that:

1) sort by their dependency.
   #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.

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