[PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path

Philippe Mathieu-Daudé posted 3 patches 9 months ago
Maintainers: Radoslaw Biernacki <rad@semihalf.com>, Peter Maydell <peter.maydell@linaro.org>, Leif Lindholm <quic_llindhol@quicinc.com>, Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>, John Snow <jsnow@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
There is a newer version of this series
[PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by Philippe Mathieu-Daudé 9 months ago
Rename "internal.h" as "ide_internal.h", and include
it via its relative local path, instead of absolute
to the project root path.

Mechanical patch doing:

  $ sed -i -e 's#hw/ide/internal.h#ide_internal.h#' \
        $(git grep -l hw/ide/internal)
  $ git mv hw/ide/internal.h hw/ide/ide_internal.h

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ide/ahci_internal.h                | 2 +-
 hw/ide/{internal.h => ide_internal.h} | 0
 hw/ide/ahci.c                         | 2 +-
 hw/ide/atapi.c                        | 2 +-
 hw/ide/cmd646.c                       | 2 +-
 hw/ide/core.c                         | 2 +-
 hw/ide/ide-bus.c                      | 2 +-
 hw/ide/ide-dev.c                      | 2 +-
 hw/ide/ioport.c                       | 2 +-
 hw/ide/isa.c                          | 2 +-
 hw/ide/macio.c                        | 2 +-
 hw/ide/microdrive.c                   | 2 +-
 hw/ide/mmio.c                         | 2 +-
 hw/ide/pci.c                          | 2 +-
 hw/ide/piix.c                         | 2 +-
 hw/ide/sii3112.c                      | 2 +-
 hw/ide/via.c                          | 2 +-
 17 files changed, 16 insertions(+), 16 deletions(-)
 rename hw/ide/{internal.h => ide_internal.h} (100%)

diff --git a/hw/ide/ahci_internal.h b/hw/ide/ahci_internal.h
index 4e13329bb2..34f3d50ca9 100644
--- a/hw/ide/ahci_internal.h
+++ b/hw/ide/ahci_internal.h
@@ -25,7 +25,7 @@
 #define HW_IDE_AHCI_INTERNAL_H
 
 #include "hw/ide/ahci.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/pci/pci_device.h"
 
 #define AHCI_MEM_BAR_SIZE         0x1000
diff --git a/hw/ide/internal.h b/hw/ide/ide_internal.h
similarity index 100%
rename from hw/ide/internal.h
rename to hw/ide/ide_internal.h
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 54c9685495..c2af344ac5 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -34,7 +34,7 @@
 #include "qemu/module.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/dma.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci-pci.h"
 #include "hw/ide/ahci-sysbus.h"
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index dcc39df9a4..8ac99f64d2 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -24,7 +24,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"
 #include "scsi/constants.h"
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 23d213ff01..be72f4d3ed 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,7 +33,7 @@
 #include "sysemu/reset.h"
 
 #include "hw/ide/pci.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "trace.h"
 
 /* CMD646 specific */
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 9c4a812902..8f2004f93b 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -41,7 +41,7 @@
 #include "qemu/cutils.h"
 #include "sysemu/replay.h"
 #include "sysemu/runstate.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "trace.h"
 
 /* These values were based on a Seagate ST3500418AS but have been modified
diff --git a/hw/ide/ide-bus.c b/hw/ide/ide-bus.c
index 57fe67b29c..32e9f62137 100644
--- a/hw/ide/ide-bus.c
+++ b/hw/ide/ide-bus.c
@@ -21,7 +21,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/runstate.h"
diff --git a/hw/ide/ide-dev.c b/hw/ide/ide-dev.c
index c8e2033469..832293ca69 100644
--- a/hw/ide/ide-dev.c
+++ b/hw/ide/ide-dev.c
@@ -23,7 +23,7 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "hw/ide/ide-dev.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
index 0b283ac783..7a80c549ec 100644
--- a/hw/ide/ioport.c
+++ b/hw/ide/ioport.c
@@ -25,7 +25,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/isa/isa.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "trace.h"
 
 int ide_init_ioport(IDEBus *bus, ISADevice *dev, int iobase, int iobase2)
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index cc865c83dc..2fc28d64b6 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -32,7 +32,7 @@
 #include "sysemu/dma.h"
 
 #include "hw/ide/isa.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "qom/object.h"
 
 /***********************************************************/
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 0d2c6ba910..7611a99ef5 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -33,7 +33,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/dma.h"
 
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 
 /* debug MACIO */
 // #define DEBUG_MACIO
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index a7f415f0fc..09781658f9 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -31,7 +31,7 @@
 #include "sysemu/dma.h"
 #include "hw/irq.h"
 
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "qom/object.h"
 
 #define TYPE_MICRODRIVE "microdrive"
diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
index e8f41c0610..1ba2541b40 100644
--- a/hw/ide/mmio.c
+++ b/hw/ide/mmio.c
@@ -30,7 +30,7 @@
 #include "sysemu/dma.h"
 
 #include "hw/ide/mmio.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/qdev-properties.h"
 
 /***********************************************************/
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 73efeec7f4..855cbb031d 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -30,7 +30,7 @@
 #include "sysemu/dma.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/ide/pci.h"
 #include "trace.h"
 
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 1773a068c3..886cf840f1 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -30,7 +30,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/pci/pci.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/ide/piix.h"
 #include "hw/ide/pci.h"
 #include "trace.h"
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 321b9e46a1..e28bbd160d 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -13,7 +13,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/ide/pci.h"
 #include "qemu/module.h"
 #include "trace.h"
diff --git a/hw/ide/via.c b/hw/ide/via.c
index cf151e70ec..ceecb3aa01 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -25,7 +25,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/ide/internal.h"
+#include "ide_internal.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
-- 
2.41.0


Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by BALATON Zoltan 9 months ago
On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
> Rename "internal.h" as "ide_internal.h", and include

Is there a convention about using underscore or dash in file names? The 
headers Thomas added are using - as well as ahci-allwinner.c, only 
ahci_internal.h has _ (but there are others elsewhere such as 
pci_device.h). Maybe we should be consistent at least within IDE and this 
series is now a good opportunity for renaming these headers to match. But 
it's just a small nit, thanks for picking this up.

Regards,
BALATON Zoltan

> it via its relative local path, instead of absolute
> to the project root path.
>
> Mechanical patch doing:
>
>  $ sed -i -e 's#hw/ide/internal.h#ide_internal.h#' \
>        $(git grep -l hw/ide/internal)
>  $ git mv hw/ide/internal.h hw/ide/ide_internal.h
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ide/ahci_internal.h                | 2 +-
> hw/ide/{internal.h => ide_internal.h} | 0
> hw/ide/ahci.c                         | 2 +-
> hw/ide/atapi.c                        | 2 +-
> hw/ide/cmd646.c                       | 2 +-
> hw/ide/core.c                         | 2 +-
> hw/ide/ide-bus.c                      | 2 +-
> hw/ide/ide-dev.c                      | 2 +-
> hw/ide/ioport.c                       | 2 +-
> hw/ide/isa.c                          | 2 +-
> hw/ide/macio.c                        | 2 +-
> hw/ide/microdrive.c                   | 2 +-
> hw/ide/mmio.c                         | 2 +-
> hw/ide/pci.c                          | 2 +-
> hw/ide/piix.c                         | 2 +-
> hw/ide/sii3112.c                      | 2 +-
> hw/ide/via.c                          | 2 +-
> 17 files changed, 16 insertions(+), 16 deletions(-)
> rename hw/ide/{internal.h => ide_internal.h} (100%)
>
> diff --git a/hw/ide/ahci_internal.h b/hw/ide/ahci_internal.h
> index 4e13329bb2..34f3d50ca9 100644
> --- a/hw/ide/ahci_internal.h
> +++ b/hw/ide/ahci_internal.h
> @@ -25,7 +25,7 @@
> #define HW_IDE_AHCI_INTERNAL_H
>
> #include "hw/ide/ahci.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/pci/pci_device.h"
>
> #define AHCI_MEM_BAR_SIZE         0x1000
> diff --git a/hw/ide/internal.h b/hw/ide/ide_internal.h
> similarity index 100%
> rename from hw/ide/internal.h
> rename to hw/ide/ide_internal.h
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 54c9685495..c2af344ac5 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -34,7 +34,7 @@
> #include "qemu/module.h"
> #include "sysemu/block-backend.h"
> #include "sysemu/dma.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/ide/pci.h"
> #include "hw/ide/ahci-pci.h"
> #include "hw/ide/ahci-sysbus.h"
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index dcc39df9a4..8ac99f64d2 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -24,7 +24,7 @@
>  */
>
> #include "qemu/osdep.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/scsi/scsi.h"
> #include "sysemu/block-backend.h"
> #include "scsi/constants.h"
> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
> index 23d213ff01..be72f4d3ed 100644
> --- a/hw/ide/cmd646.c
> +++ b/hw/ide/cmd646.c
> @@ -33,7 +33,7 @@
> #include "sysemu/reset.h"
>
> #include "hw/ide/pci.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "trace.h"
>
> /* CMD646 specific */
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 9c4a812902..8f2004f93b 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -41,7 +41,7 @@
> #include "qemu/cutils.h"
> #include "sysemu/replay.h"
> #include "sysemu/runstate.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "trace.h"
>
> /* These values were based on a Seagate ST3500418AS but have been modified
> diff --git a/hw/ide/ide-bus.c b/hw/ide/ide-bus.c
> index 57fe67b29c..32e9f62137 100644
> --- a/hw/ide/ide-bus.c
> +++ b/hw/ide/ide-bus.c
> @@ -21,7 +21,7 @@
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "qemu/module.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "sysemu/block-backend.h"
> #include "sysemu/blockdev.h"
> #include "sysemu/runstate.h"
> diff --git a/hw/ide/ide-dev.c b/hw/ide/ide-dev.c
> index c8e2033469..832293ca69 100644
> --- a/hw/ide/ide-dev.c
> +++ b/hw/ide/ide-dev.c
> @@ -23,7 +23,7 @@
> #include "qemu/error-report.h"
> #include "qemu/module.h"
> #include "hw/ide/ide-dev.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "sysemu/block-backend.h"
> #include "sysemu/blockdev.h"
> #include "sysemu/sysemu.h"
> diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
> index 0b283ac783..7a80c549ec 100644
> --- a/hw/ide/ioport.c
> +++ b/hw/ide/ioport.c
> @@ -25,7 +25,7 @@
>
> #include "qemu/osdep.h"
> #include "hw/isa/isa.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "trace.h"
>
> int ide_init_ioport(IDEBus *bus, ISADevice *dev, int iobase, int iobase2)
> diff --git a/hw/ide/isa.c b/hw/ide/isa.c
> index cc865c83dc..2fc28d64b6 100644
> --- a/hw/ide/isa.c
> +++ b/hw/ide/isa.c
> @@ -32,7 +32,7 @@
> #include "sysemu/dma.h"
>
> #include "hw/ide/isa.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "qom/object.h"
>
> /***********************************************************/
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index 0d2c6ba910..7611a99ef5 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -33,7 +33,7 @@
> #include "sysemu/block-backend.h"
> #include "sysemu/dma.h"
>
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
>
> /* debug MACIO */
> // #define DEBUG_MACIO
> diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
> index a7f415f0fc..09781658f9 100644
> --- a/hw/ide/microdrive.c
> +++ b/hw/ide/microdrive.c
> @@ -31,7 +31,7 @@
> #include "sysemu/dma.h"
> #include "hw/irq.h"
>
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "qom/object.h"
>
> #define TYPE_MICRODRIVE "microdrive"
> diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
> index e8f41c0610..1ba2541b40 100644
> --- a/hw/ide/mmio.c
> +++ b/hw/ide/mmio.c
> @@ -30,7 +30,7 @@
> #include "sysemu/dma.h"
>
> #include "hw/ide/mmio.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/qdev-properties.h"
>
> /***********************************************************/
> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
> index 73efeec7f4..855cbb031d 100644
> --- a/hw/ide/pci.c
> +++ b/hw/ide/pci.c
> @@ -30,7 +30,7 @@
> #include "sysemu/dma.h"
> #include "qemu/error-report.h"
> #include "qemu/module.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/ide/pci.h"
> #include "trace.h"
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 1773a068c3..886cf840f1 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -30,7 +30,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "hw/pci/pci.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/ide/piix.h"
> #include "hw/ide/pci.h"
> #include "trace.h"
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> index 321b9e46a1..e28bbd160d 100644
> --- a/hw/ide/sii3112.c
> +++ b/hw/ide/sii3112.c
> @@ -13,7 +13,7 @@
>  */
>
> #include "qemu/osdep.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/ide/pci.h"
> #include "qemu/module.h"
> #include "trace.h"
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index cf151e70ec..ceecb3aa01 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -25,7 +25,7 @@
>  */
>
> #include "qemu/osdep.h"
> -#include "hw/ide/internal.h"
> +#include "ide_internal.h"
> #include "hw/pci/pci.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
>
Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by Markus Armbruster 9 months ago
BALATON Zoltan <balaton@eik.bme.hu> writes:

> On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
>> Rename "internal.h" as "ide_internal.h", and include
>
> Is there a convention about using underscore or dash in file names? The headers Thomas added are using - as well as ahci-allwinner.c, only ahci_internal.h has _ (but there are others elsewhere such as pci_device.h). Maybe we should be consistent at least within IDE and this series is now a good opportunity for renaming these headers to match. But it's just a small nit, thanks for picking this up.

This is one of the many unnecessary inconsistencies we're inflicting on
ourselves.

We have more than 3600 file names containing '-', and more almost 2700
containing '_'.  Bizarrely, 68 of them contain both.

I strongly prefer '_' myself.

Zoltan is making a local consistency argument for '-'.

Let's use '-' here.
Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by Philippe Mathieu-Daudé 9 months ago
On 26/2/24 08:40, Markus Armbruster wrote:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
> 
>> On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
>>> Rename "internal.h" as "ide_internal.h", and include
>>
>> Is there a convention about using underscore or dash in file names? The headers Thomas added are using - as well as ahci-allwinner.c, only ahci_internal.h has _ (but there are others elsewhere such as pci_device.h). Maybe we should be consistent at least within IDE and this series is now a good opportunity for renaming these headers to match. But it's just a small nit, thanks for picking this up.
> 
> This is one of the many unnecessary inconsistencies we're inflicting on
> ourselves.
> 
> We have more than 3600 file names containing '-', and more almost 2700
> containing '_'.  Bizarrely, 68 of them contain both.
> 
> I strongly prefer '_' myself.
> 
> Zoltan is making a local consistency argument for '-'.
> 
> Let's use '-' here.

Fine, patch updated.


Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by BALATON Zoltan 9 months ago
On Mon, 26 Feb 2024, Philippe Mathieu-Daudé wrote:
> On 26/2/24 08:40, Markus Armbruster wrote:
>> BALATON Zoltan <balaton@eik.bme.hu> writes:
>> 
>>> On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
>>>> Rename "internal.h" as "ide_internal.h", and include
>>> 
>>> Is there a convention about using underscore or dash in file names? The 
>>> headers Thomas added are using - as well as ahci-allwinner.c, only 
>>> ahci_internal.h has _ (but there are others elsewhere such as 
>>> pci_device.h). Maybe we should be consistent at least within IDE and this 
>>> series is now a good opportunity for renaming these headers to match. But 
>>> it's just a small nit, thanks for picking this up.
>> 
>> This is one of the many unnecessary inconsistencies we're inflicting on
>> ourselves.
>> 
>> We have more than 3600 file names containing '-', and more almost 2700
>> containing '_'.  Bizarrely, 68 of them contain both.
>> 
>> I strongly prefer '_' myself.
>> 
>> Zoltan is making a local consistency argument for '-'.
>> 
>> Let's use '-' here.
>
> Fine, patch updated.

And then please also rename ahci_internal.h to use '-' to be really 
consistent.

Regards,
BALATON Zoltan
Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by Philippe Mathieu-Daudé 9 months ago
On 26/2/24 11:50, BALATON Zoltan wrote:
> On Mon, 26 Feb 2024, Philippe Mathieu-Daudé wrote:
>> On 26/2/24 08:40, Markus Armbruster wrote:
>>> BALATON Zoltan <balaton@eik.bme.hu> writes:
>>>
>>>> On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
>>>>> Rename "internal.h" as "ide_internal.h", and include
>>>>
>>>> Is there a convention about using underscore or dash in file names? 
>>>> The headers Thomas added are using - as well as ahci-allwinner.c, 
>>>> only ahci_internal.h has _ (but there are others elsewhere such as 
>>>> pci_device.h). Maybe we should be consistent at least within IDE and 
>>>> this series is now a good opportunity for renaming these headers to 
>>>> match. But it's just a small nit, thanks for picking this up.
>>>
>>> This is one of the many unnecessary inconsistencies we're inflicting on
>>> ourselves.
>>>
>>> We have more than 3600 file names containing '-', and more almost 2700
>>> containing '_'.  Bizarrely, 68 of them contain both.
>>>
>>> I strongly prefer '_' myself.
>>>
>>> Zoltan is making a local consistency argument for '-'.
>>>
>>> Let's use '-' here.
>>
>> Fine, patch updated.
> 
> And then please also rename ahci_internal.h to use '-' to be really 
> consistent.

I'm sorry but I don't have time. Maybe fill a ByteSized task?


Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by BALATON Zoltan 9 months ago
On Mon, 26 Feb 2024, Philippe Mathieu-Daudé wrote:
> On 26/2/24 11:50, BALATON Zoltan wrote:
>> On Mon, 26 Feb 2024, Philippe Mathieu-Daudé wrote:
>>> On 26/2/24 08:40, Markus Armbruster wrote:
>>>> BALATON Zoltan <balaton@eik.bme.hu> writes:
>>>> 
>>>>> On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
>>>>>> Rename "internal.h" as "ide_internal.h", and include
>>>>> 
>>>>> Is there a convention about using underscore or dash in file names? The 
>>>>> headers Thomas added are using - as well as ahci-allwinner.c, only 
>>>>> ahci_internal.h has _ (but there are others elsewhere such as 
>>>>> pci_device.h). Maybe we should be consistent at least within IDE and 
>>>>> this series is now a good opportunity for renaming these headers to 
>>>>> match. But it's just a small nit, thanks for picking this up.
>>>> 
>>>> This is one of the many unnecessary inconsistencies we're inflicting on
>>>> ourselves.
>>>> 
>>>> We have more than 3600 file names containing '-', and more almost 2700
>>>> containing '_'.  Bizarrely, 68 of them contain both.
>>>> 
>>>> I strongly prefer '_' myself.
>>>> 
>>>> Zoltan is making a local consistency argument for '-'.
>>>> 
>>>> Let's use '-' here.
>>> 
>>> Fine, patch updated.
>> 
>> And then please also rename ahci_internal.h to use '-' to be really 
>> consistent.
>
> I'm sorry but I don't have time. Maybe fill a ByteSized task?

No problem, just noted in case you'd respin it for some other reason. 
Otherwise somebody can send a patch later.

Regards,
BALATON Zoltan
Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by Philippe Mathieu-Daudé 9 months ago
On 26/2/24 09:00, Philippe Mathieu-Daudé wrote:
> On 26/2/24 08:40, Markus Armbruster wrote:
>> BALATON Zoltan <balaton@eik.bme.hu> writes:
>>
>>> On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
>>>> Rename "internal.h" as "ide_internal.h", and include
>>>
>>> Is there a convention about using underscore or dash in file names? 
>>> The headers Thomas added are using - as well as ahci-allwinner.c, 
>>> only ahci_internal.h has _ (but there are others elsewhere such as 
>>> pci_device.h). Maybe we should be consistent at least within IDE and 
>>> this series is now a good opportunity for renaming these headers to 
>>> match. But it's just a small nit, thanks for picking this up.
>>
>> This is one of the many unnecessary inconsistencies we're inflicting on
>> ourselves.
>>
>> We have more than 3600 file names containing '-', and more almost 2700
>> containing '_'.  Bizarrely, 68 of them contain both.
>>
>> I strongly prefer '_' myself.
>>
>> Zoltan is making a local consistency argument for '-'.
>>
>> Let's use '-' here.

BTW by "here" I understood "in this patch". After v3 only a single
file use '_': "ahci_internal.h", but Zoltan said he is uninterested
and doesn't care about it so I'm not renaming it.
(https://lore.kernel.org/qemu-devel/3e576d33-1973-3f44-afd1-9224609f89a6@eik.bme.hu/)

> 
> Fine, patch updated.
> 


Re: [PATCH v2 3/3] hw/ide: Include 'ide_internal.h' from current path
Posted by BALATON Zoltan 9 months ago
On Mon, 26 Feb 2024, Philippe Mathieu-Daudé wrote:
> On 26/2/24 09:00, Philippe Mathieu-Daudé wrote:
>> On 26/2/24 08:40, Markus Armbruster wrote:
>>> BALATON Zoltan <balaton@eik.bme.hu> writes:
>>> 
>>>> On Sun, 25 Feb 2024, Philippe Mathieu-Daudé wrote:
>>>>> Rename "internal.h" as "ide_internal.h", and include
>>>> 
>>>> Is there a convention about using underscore or dash in file names? The 
>>>> headers Thomas added are using - as well as ahci-allwinner.c, only 
>>>> ahci_internal.h has _ (but there are others elsewhere such as 
>>>> pci_device.h). Maybe we should be consistent at least within IDE and this 
>>>> series is now a good opportunity for renaming these headers to match. But 
>>>> it's just a small nit, thanks for picking this up.
>>> 
>>> This is one of the many unnecessary inconsistencies we're inflicting on
>>> ourselves.
>>> 
>>> We have more than 3600 file names containing '-', and more almost 2700
>>> containing '_'.  Bizarrely, 68 of them contain both.
>>> 
>>> I strongly prefer '_' myself.
>>> 
>>> Zoltan is making a local consistency argument for '-'.
>>> 
>>> Let's use '-' here.
>
> BTW by "here" I understood "in this patch". After v3 only a single
> file use '_': "ahci_internal.h", but Zoltan said he is uninterested
> and doesn't care about it so I'm not renaming it.
> (https://lore.kernel.org/qemu-devel/3e576d33-1973-3f44-afd1-9224609f89a6@eik.bme.hu/)

Yes, I don't care and also prefer _ in file names but since all the other 
headers use - now it's simpler to make all of them the same just for 
consistency so I've meant to rename ahci_internal.h too but forgot to 
mention it explikcitly. Sorry for that.

Regards,
BALATON Zoltan