* [PATCH][next] nvdimm: nd.h: Replace zero-length array with flexible-array member
@ 2020-03-19 23:09 ` Gustavo A. R. Silva
0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-19 23:09 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny
Cc: linux-nvdimm, 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/nvdimm/nd.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index c4d69c1cce55..85dbb2a322b9 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -39,7 +39,7 @@ struct nd_region_data {
int ns_count;
int ns_active;
unsigned int hints_shift;
- void __iomem *flush_wpq[0];
+ void __iomem *flush_wpq[];
};
static inline void __iomem *ndrd_get_flush_wpq(struct nd_region_data *ndrd,
@@ -157,7 +157,7 @@ struct nd_region {
struct nd_interleave_set *nd_set;
struct nd_percpu_lane __percpu *lane;
int (*flush)(struct nd_region *nd_region, struct bio *bio);
- struct nd_mapping mapping[0];
+ struct nd_mapping mapping[];
};
struct nd_blk_region {
--
2.23.0
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH][next] nvdimm: nd.h: Replace zero-length array with flexible-array member
@ 2020-03-19 23:09 ` Gustavo A. R. Silva
0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-19 23:09 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny
Cc: linux-nvdimm, 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/nvdimm/nd.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index c4d69c1cce55..85dbb2a322b9 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -39,7 +39,7 @@ struct nd_region_data {
int ns_count;
int ns_active;
unsigned int hints_shift;
- void __iomem *flush_wpq[0];
+ void __iomem *flush_wpq[];
};
static inline void __iomem *ndrd_get_flush_wpq(struct nd_region_data *ndrd,
@@ -157,7 +157,7 @@ struct nd_region {
struct nd_interleave_set *nd_set;
struct nd_percpu_lane __percpu *lane;
int (*flush)(struct nd_region *nd_region, struct bio *bio);
- struct nd_mapping mapping[0];
+ struct nd_mapping mapping[];
};
struct nd_blk_region {
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH][next] nvdimm: nd.h: Replace zero-length array with flexible-array member
2020-03-19 23:09 ` Gustavo A. R. Silva
@ 2020-03-19 23:47 ` Ira Weiny
-1 siblings, 0 replies; 6+ messages in thread
From: Ira Weiny @ 2020-03-19 23:47 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: linux-nvdimm, linux-kernel
On Thu, Mar 19, 2020 at 06:09:37PM -0500, Gustavo A. R. Silva wrote:
> 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:
"won't" be affected?
My reading of [1] indicates that this change will break the allocation in
nd_region_activate() because sizeof() can no longer be used on the base
structure?
What am I missing?
Ira
>
> "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/nvdimm/nd.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index c4d69c1cce55..85dbb2a322b9 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -39,7 +39,7 @@ struct nd_region_data {
> int ns_count;
> int ns_active;
> unsigned int hints_shift;
> - void __iomem *flush_wpq[0];
> + void __iomem *flush_wpq[];
> };
>
> static inline void __iomem *ndrd_get_flush_wpq(struct nd_region_data *ndrd,
> @@ -157,7 +157,7 @@ struct nd_region {
> struct nd_interleave_set *nd_set;
> struct nd_percpu_lane __percpu *lane;
> int (*flush)(struct nd_region *nd_region, struct bio *bio);
> - struct nd_mapping mapping[0];
> + struct nd_mapping mapping[];
> };
>
> struct nd_blk_region {
> --
> 2.23.0
>
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH][next] nvdimm: nd.h: Replace zero-length array with flexible-array member
@ 2020-03-19 23:47 ` Ira Weiny
0 siblings, 0 replies; 6+ messages in thread
From: Ira Weiny @ 2020-03-19 23:47 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Dan Williams, Vishal Verma, Dave Jiang, linux-nvdimm,
linux-kernel
On Thu, Mar 19, 2020 at 06:09:37PM -0500, Gustavo A. R. Silva wrote:
> 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:
"won't" be affected?
My reading of [1] indicates that this change will break the allocation in
nd_region_activate() because sizeof() can no longer be used on the base
structure?
What am I missing?
Ira
>
> "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/nvdimm/nd.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index c4d69c1cce55..85dbb2a322b9 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -39,7 +39,7 @@ struct nd_region_data {
> int ns_count;
> int ns_active;
> unsigned int hints_shift;
> - void __iomem *flush_wpq[0];
> + void __iomem *flush_wpq[];
> };
>
> static inline void __iomem *ndrd_get_flush_wpq(struct nd_region_data *ndrd,
> @@ -157,7 +157,7 @@ struct nd_region {
> struct nd_interleave_set *nd_set;
> struct nd_percpu_lane __percpu *lane;
> int (*flush)(struct nd_region *nd_region, struct bio *bio);
> - struct nd_mapping mapping[0];
> + struct nd_mapping mapping[];
> };
>
> struct nd_blk_region {
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH][next] nvdimm: nd.h: Replace zero-length array with flexible-array member
2020-03-19 23:47 ` Ira Weiny
@ 2020-03-20 0:03 ` Gustavo A. R. Silva
-1 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-20 0:03 UTC (permalink / raw)
To: Ira Weiny; +Cc: linux-nvdimm, linux-kernel
On 3/19/20 6:47 PM, Ira Weiny wrote:
> On Thu, Mar 19, 2020 at 06:09:37PM -0500, Gustavo A. R. Silva wrote:
>> 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:
>
> "won't" be affected?
>
Yep. They won't:
"As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."
> My reading of [1] indicates that this change will break the allocation inha
The allocation will remain the same as with *flush_wpg[0]. See this example: https://godbolt.org/z/oohwmB
> nd_region_activate() because sizeof() can no longer be used on the base
> structure?
>
In this case the sizeof operator cannot be applied to flush_wpg, not the base structure, which is
nd_region_data.
> What am I missing?
>
I think you are confusing the base structure with the actual flexible-array member.
Thanks
--
Gustavo
> Ira
>
>>
>> "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/nvdimm/nd.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
>> index c4d69c1cce55..85dbb2a322b9 100644
>> --- a/drivers/nvdimm/nd.h
>> +++ b/drivers/nvdimm/nd.h
>> @@ -39,7 +39,7 @@ struct nd_region_data {
>> int ns_count;
>> int ns_active;
>> unsigned int hints_shift;
>> - void __iomem *flush_wpq[0];
>> + void __iomem *flush_wpq[];
>> };
>>
>> static inline void __iomem *ndrd_get_flush_wpq(struct nd_region_data *ndrd,
>> @@ -157,7 +157,7 @@ struct nd_region {
>> struct nd_interleave_set *nd_set;
>> struct nd_percpu_lane __percpu *lane;
>> int (*flush)(struct nd_region *nd_region, struct bio *bio);
>> - struct nd_mapping mapping[0];
>> + struct nd_mapping mapping[];
>> };
>>
>> struct nd_blk_region {
>> --
>> 2.23.0
>>
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH][next] nvdimm: nd.h: Replace zero-length array with flexible-array member
@ 2020-03-20 0:03 ` Gustavo A. R. Silva
0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-20 0:03 UTC (permalink / raw)
To: Ira Weiny
Cc: Dan Williams, Vishal Verma, Dave Jiang, linux-nvdimm,
linux-kernel
On 3/19/20 6:47 PM, Ira Weiny wrote:
> On Thu, Mar 19, 2020 at 06:09:37PM -0500, Gustavo A. R. Silva wrote:
>> 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:
>
> "won't" be affected?
>
Yep. They won't:
"As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."
> My reading of [1] indicates that this change will break the allocation inha
The allocation will remain the same as with *flush_wpg[0]. See this example: https://godbolt.org/z/oohwmB
> nd_region_activate() because sizeof() can no longer be used on the base
> structure?
>
In this case the sizeof operator cannot be applied to flush_wpg, not the base structure, which is
nd_region_data.
> What am I missing?
>
I think you are confusing the base structure with the actual flexible-array member.
Thanks
--
Gustavo
> Ira
>
>>
>> "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/nvdimm/nd.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
>> index c4d69c1cce55..85dbb2a322b9 100644
>> --- a/drivers/nvdimm/nd.h
>> +++ b/drivers/nvdimm/nd.h
>> @@ -39,7 +39,7 @@ struct nd_region_data {
>> int ns_count;
>> int ns_active;
>> unsigned int hints_shift;
>> - void __iomem *flush_wpq[0];
>> + void __iomem *flush_wpq[];
>> };
>>
>> static inline void __iomem *ndrd_get_flush_wpq(struct nd_region_data *ndrd,
>> @@ -157,7 +157,7 @@ struct nd_region {
>> struct nd_interleave_set *nd_set;
>> struct nd_percpu_lane __percpu *lane;
>> int (*flush)(struct nd_region *nd_region, struct bio *bio);
>> - struct nd_mapping mapping[0];
>> + struct nd_mapping mapping[];
>> };
>>
>> struct nd_blk_region {
>> --
>> 2.23.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-20 0:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-19 23:09 [PATCH][next] nvdimm: nd.h: Replace zero-length array with flexible-array member Gustavo A. R. Silva
2020-03-19 23:09 ` Gustavo A. R. Silva
2020-03-19 23:47 ` Ira Weiny
2020-03-19 23:47 ` Ira Weiny
2020-03-20 0:03 ` Gustavo A. R. Silva
2020-03-20 0:03 ` 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.