All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: hv: Replace zero-length array with flexible-array member
@ 2020-02-13  0:50 Gustavo A. R. Silva
  2020-02-13  3:43 ` Dexuan Cui
  0 siblings, 1 reply; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-02-13  0:50 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas
  Cc: linux-hyperv, linux-pci, linux-kernel, Gustavo A. R. Silva

The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by
this change:

"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/pci/controller/pci-hyperv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 9977abff92fc..be957268f9d6 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -260,7 +260,7 @@ struct pci_packet {
 				int resp_packet_size);
 	void *compl_ctxt;
 
-	struct pci_message message[0];
+	struct pci_message message[];
 };
 
 /*
@@ -296,7 +296,7 @@ struct pci_bus_d0_entry {
 struct pci_bus_relations {
 	struct pci_incoming_message incoming;
 	u32 device_count;
-	struct pci_function_description func[0];
+	struct pci_function_description func[];
 } __packed;
 
 struct pci_q_res_req_response {
@@ -508,7 +508,7 @@ struct hv_dr_work {
 struct hv_dr_state {
 	struct list_head list_entry;
 	u32 device_count;
-	struct pci_function_description func[0];
+	struct pci_function_description func[];
 };
 
 enum hv_pcichild_state {
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
  2020-02-13  0:50 [PATCH] PCI: hv: Replace zero-length array with flexible-array member Gustavo A. R. Silva
@ 2020-02-13  3:43 ` Dexuan Cui
  2020-03-04 17:55   ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Dexuan Cui @ 2020-02-13  3:43 UTC (permalink / raw)
  To: Gustavo A. R. Silva, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
	Bjorn Helgaas
  Cc: linux-hyperv, linux-pci, linux-kernel

> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Gustavo A. R. Silva
> Sent: Wednesday, February 12, 2020 4:51 PM
>  ...
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
> 
> struct foo {
>         int stuff;
>         struct boo array[];
> };
> 
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
> 
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
> 
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
> 
> This issue was found with the help of Coccinelle.

Looks good to me. Thanks, Gustavo!
 
Reviewed-by: Dexuan Cui <decui@microsoft.com>

FWIW, it looks there are a lot of more to fix in the kernel tree: the below
commands return 1373 for me:

grep -nr '\[0\];$' * | grep '\.h:' | grep -v = | wc -l

Running the commands against the kernel/ directory returns 3.

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
  2020-02-13  3:43 ` Dexuan Cui
@ 2020-03-04 17:55   ` Wei Liu
  2020-03-04 18:06     ` Lorenzo Pieralisi
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2020-03-04 17:55 UTC (permalink / raw)
  To: Dexuan Cui, Lorenzo Pieralisi
  Cc: Gustavo A. R. Silva, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Sasha Levin, Lorenzo Pieralisi, Andrew Murray,
	Bjorn Helgaas, linux-hyperv, linux-pci, linux-kernel, Wei Liu

On Thu, Feb 13, 2020 at 03:43:40AM +0000, Dexuan Cui wrote:
> > From: linux-hyperv-owner@vger.kernel.org
> > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Gustavo A. R. Silva
> > Sent: Wednesday, February 12, 2020 4:51 PM
> >  ...
> > The current codebase makes use of the zero-length array language
> > extension to the C90 standard, but the preferred mechanism to declare
> > variable-length types such as these ones is a flexible array member[1][2],
> > introduced in C99:
> > 
> > struct foo {
> >         int stuff;
> >         struct boo array[];
> > };
> > 
> > By making use of the mechanism above, we will get a compiler warning
> > in case the flexible array does not occur last in the structure, which
> > will help us prevent some kind of undefined behavior bugs from being
> > inadvertently introduced[3] to the codebase from now on.
> > 
> > Also, notice that, dynamic memory allocations won't be affected by
> > this change:
> > 
> > "Flexible array members have incomplete type, and so the sizeof operator
> > may not be applied. As a quirk of the original implementation of
> > zero-length arrays, sizeof evaluates to zero."[1]
> > 
> > This issue was found with the help of Coccinelle.
> 
> Looks good to me. Thanks, Gustavo!
>  
> Reviewed-by: Dexuan Cui <decui@microsoft.com>
> 

Lorenzo, will you be picking up this patch? It seems to me you've been
handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
in your repository.

Let me know what you think.

Wei.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
  2020-03-04 17:55   ` Wei Liu
@ 2020-03-04 18:06     ` Lorenzo Pieralisi
  2020-03-04 18:10       ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Lorenzo Pieralisi @ 2020-03-04 18:06 UTC (permalink / raw)
  To: Wei Liu
  Cc: Dexuan Cui, Gustavo A. R. Silva, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Sasha Levin, Andrew Murray, Bjorn Helgaas,
	linux-hyperv, linux-pci, linux-kernel

On Wed, Mar 04, 2020 at 05:55:09PM +0000, Wei Liu wrote:
> On Thu, Feb 13, 2020 at 03:43:40AM +0000, Dexuan Cui wrote:
> > > From: linux-hyperv-owner@vger.kernel.org
> > > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Gustavo A. R. Silva
> > > Sent: Wednesday, February 12, 2020 4:51 PM
> > >  ...
> > > The current codebase makes use of the zero-length array language
> > > extension to the C90 standard, but the preferred mechanism to declare
> > > variable-length types such as these ones is a flexible array member[1][2],
> > > introduced in C99:
> > > 
> > > struct foo {
> > >         int stuff;
> > >         struct boo array[];
> > > };
> > > 
> > > By making use of the mechanism above, we will get a compiler warning
> > > in case the flexible array does not occur last in the structure, which
> > > will help us prevent some kind of undefined behavior bugs from being
> > > inadvertently introduced[3] to the codebase from now on.
> > > 
> > > Also, notice that, dynamic memory allocations won't be affected by
> > > this change:
> > > 
> > > "Flexible array members have incomplete type, and so the sizeof operator
> > > may not be applied. As a quirk of the original implementation of
> > > zero-length arrays, sizeof evaluates to zero."[1]
> > > 
> > > This issue was found with the help of Coccinelle.
> > 
> > Looks good to me. Thanks, Gustavo!
> >  
> > Reviewed-by: Dexuan Cui <decui@microsoft.com>
> > 
> 
> Lorenzo, will you be picking up this patch? It seems to me you've been
> handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
> in your repository.
> 
> Let me know what you think.

I shall pick it up, I checked patchwork and it was erroneously
assigned to Bjorn, that's why I have not taken it yet.

Fixed now, apologies, I will merge it shortly.

Lorenzo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
  2020-03-04 18:06     ` Lorenzo Pieralisi
@ 2020-03-04 18:10       ` Wei Liu
  2020-03-04 18:24         ` Gustavo A. R. Silva
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2020-03-04 18:10 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Wei Liu, Dexuan Cui, Gustavo A. R. Silva, KY Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Andrew Murray,
	Bjorn Helgaas, linux-hyperv, linux-pci, linux-kernel

On Wed, Mar 04, 2020 at 06:06:35PM +0000, Lorenzo Pieralisi wrote:
> On Wed, Mar 04, 2020 at 05:55:09PM +0000, Wei Liu wrote:
> > On Thu, Feb 13, 2020 at 03:43:40AM +0000, Dexuan Cui wrote:
> > > > From: linux-hyperv-owner@vger.kernel.org
> > > > <linux-hyperv-owner@vger.kernel.org> On Behalf Of Gustavo A. R. Silva
> > > > Sent: Wednesday, February 12, 2020 4:51 PM
> > > >  ...
> > > > The current codebase makes use of the zero-length array language
> > > > extension to the C90 standard, but the preferred mechanism to declare
> > > > variable-length types such as these ones is a flexible array member[1][2],
> > > > introduced in C99:
> > > > 
> > > > struct foo {
> > > >         int stuff;
> > > >         struct boo array[];
> > > > };
> > > > 
> > > > By making use of the mechanism above, we will get a compiler warning
> > > > in case the flexible array does not occur last in the structure, which
> > > > will help us prevent some kind of undefined behavior bugs from being
> > > > inadvertently introduced[3] to the codebase from now on.
> > > > 
> > > > Also, notice that, dynamic memory allocations won't be affected by
> > > > this change:
> > > > 
> > > > "Flexible array members have incomplete type, and so the sizeof operator
> > > > may not be applied. As a quirk of the original implementation of
> > > > zero-length arrays, sizeof evaluates to zero."[1]
> > > > 
> > > > This issue was found with the help of Coccinelle.
> > > 
> > > Looks good to me. Thanks, Gustavo!
> > >  
> > > Reviewed-by: Dexuan Cui <decui@microsoft.com>
> > > 
> > 
> > Lorenzo, will you be picking up this patch? It seems to me you've been
> > handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
> > in your repository.
> > 
> > Let me know what you think.
> 
> I shall pick it up, I checked patchwork and it was erroneously
> assigned to Bjorn, that's why I have not taken it yet.
> 
> Fixed now, apologies, I will merge it shortly.

Thanks for picking it up.

Wei.

> 
> Lorenzo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: hv: Replace zero-length array with flexible-array member
  2020-03-04 18:10       ` Wei Liu
@ 2020-03-04 18:24         ` Gustavo A. R. Silva
  0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-04 18:24 UTC (permalink / raw)
  To: Wei Liu, Lorenzo Pieralisi
  Cc: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Sasha Levin, Andrew Murray, Bjorn Helgaas, linux-hyperv,
	linux-pci, linux-kernel



On 3/4/20 12:10, Wei Liu wrote:

>>>>
>>>> Looks good to me. Thanks, Gustavo!
>>>>  
>>>> Reviewed-by: Dexuan Cui <decui@microsoft.com>
>>>>
>>>
>>> Lorenzo, will you be picking up this patch? It seems to me you've been
>>> handling patches to pci-hyperv.c. This patch is not yet in pci/hv branch
>>> in your repository.
>>>
>>> Let me know what you think.
>>
>> I shall pick it up, I checked patchwork and it was erroneously
>> assigned to Bjorn, that's why I have not taken it yet.
>>
>> Fixed now, apologies, I will merge it shortly.
> 
> Thanks for picking it up.
> 

Thank you all, guys. :)
--
Gustavo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-04 18:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13  0:50 [PATCH] PCI: hv: Replace zero-length array with flexible-array member Gustavo A. R. Silva
2020-02-13  3:43 ` Dexuan Cui
2020-03-04 17:55   ` Wei Liu
2020-03-04 18:06     ` Lorenzo Pieralisi
2020-03-04 18:10       ` Wei Liu
2020-03-04 18:24         ` Gustavo A. R. Silva

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.