All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage
@ 2018-03-07 19:54 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-07 19:54 UTC (permalink / raw)
  To: Florian Tobias Schandinat, Bartlomiej Zolnierkiewicz
  Cc: linux-fbdev, dri-devel, linux-kernel, Gustavo A. R. Silva

In preparation to enabling -Wvla, remove VLA and replace it
with a fixed-length array instead. Also, remove variable 'len'.

Notice that no new IDs have been added in seven years.

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

diff --git a/drivers/video/fbdev/via/via_aux_vt1632.c b/drivers/video/fbdev/via/via_aux_vt1632.c
index d24f4cd..0cd0d2a 100644
--- a/drivers/video/fbdev/via/via_aux_vt1632.c
+++ b/drivers/video/fbdev/via/via_aux_vt1632.c
@@ -35,10 +35,10 @@ static void probe(struct via_aux_bus *bus, u8 addr)
 		.addr	=	addr,
 		.name	=	name};
 	/* check vendor id and device id */
-	const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
-	u8 tmp[len];
+	const u8 id[4] = {0x06, 0x11, 0x92, 0x31};
+	u8 tmp[4];
 
-	if (!via_aux_read(&drv, 0x00, tmp, len) || memcmp(id, tmp, len))
+	if (!via_aux_read(&drv, 0x00, tmp, 4) || memcmp(id, tmp, 4))
 		return;
 
 	printk(KERN_INFO "viafb: Found %s at address 0x%x\n", name, addr);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage
@ 2018-03-07 19:54 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-07 19:54 UTC (permalink / raw)
  To: Florian Tobias Schandinat, Bartlomiej Zolnierkiewicz
  Cc: linux-fbdev, dri-devel, linux-kernel, Gustavo A. R. Silva

In preparation to enabling -Wvla, remove VLA and replace it
with a fixed-length array instead. Also, remove variable 'len'.

Notice that no new IDs have been added in seven years.

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

diff --git a/drivers/video/fbdev/via/via_aux_vt1632.c b/drivers/video/fbdev/via/via_aux_vt1632.c
index d24f4cd..0cd0d2a 100644
--- a/drivers/video/fbdev/via/via_aux_vt1632.c
+++ b/drivers/video/fbdev/via/via_aux_vt1632.c
@@ -35,10 +35,10 @@ static void probe(struct via_aux_bus *bus, u8 addr)
 		.addr	=	addr,
 		.name	=	name};
 	/* check vendor id and device id */
-	const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
-	u8 tmp[len];
+	const u8 id[4] = {0x06, 0x11, 0x92, 0x31};
+	u8 tmp[4];
 
-	if (!via_aux_read(&drv, 0x00, tmp, len) || memcmp(id, tmp, len))
+	if (!via_aux_read(&drv, 0x00, tmp, 4) || memcmp(id, tmp, 4))
 		return;
 
 	printk(KERN_INFO "viafb: Found %s at address 0x%x\n", name, addr);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage
  2018-03-07 19:54 ` Gustavo A. R. Silva
  (?)
@ 2018-03-08 11:58   ` Emil Velikov
  -1 siblings, 0 replies; 7+ messages in thread
From: Emil Velikov @ 2018-03-08 11:58 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Florian Tobias Schandinat,
	Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Gustavo A. R. Silva

Hi Gustavo,

On 7 March 2018 at 19:54, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with a fixed-length array instead. Also, remove variable 'len'.
>
What is the reason behind adding -Wvla? Is there a thread some that I
can read up on?

> Notice that no new IDs have been added in seven years.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/video/fbdev/via/via_aux_vt1632.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/via/via_aux_vt1632.c b/drivers/video/fbdev/via/via_aux_vt1632.c
> index d24f4cd..0cd0d2a 100644
> --- a/drivers/video/fbdev/via/via_aux_vt1632.c
> +++ b/drivers/video/fbdev/via/via_aux_vt1632.c
> @@ -35,10 +35,10 @@ static void probe(struct via_aux_bus *bus, u8 addr)
>                 .addr   =       addr,
>                 .name   =       name};
>         /* check vendor id and device id */
> -       const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
> -       u8 tmp[len];
> +       const u8 id[4] = {0x06, 0x11, 0x92, 0x31};
> +       u8 tmp[4];
>
> -       if (!via_aux_read(&drv, 0x00, tmp, len) || memcmp(id, tmp, len))
> +       if (!via_aux_read(&drv, 0x00, tmp, 4) || memcmp(id, tmp, 4))

Generally, hard coding a bunch of numbers like that makes for
confusing and fragile code.

A lot simpler and more obvious solution is like the following.
It silences the compiler warning (-Wvla - pedantic) you while keeping
the original clarity.

        const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
-       u8 tmp[len];
+       u8 tmp[ARRAY_SIZE(id)]; // Using len causes a Wvla warning

HTH
Emil

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage
@ 2018-03-08 11:58   ` Emil Velikov
  0 siblings, 0 replies; 7+ messages in thread
From: Emil Velikov @ 2018-03-08 11:58 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Florian Tobias Schandinat,
	Linux-Kernel@Vger. Kernel. Org, ML dri-devel, Gustavo A. R. Silva

Hi Gustavo,

On 7 March 2018 at 19:54, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with a fixed-length array instead. Also, remove variable 'len'.
>
What is the reason behind adding -Wvla? Is there a thread some that I
can read up on?

> Notice that no new IDs have been added in seven years.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/video/fbdev/via/via_aux_vt1632.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/via/via_aux_vt1632.c b/drivers/video/fbdev/via/via_aux_vt1632.c
> index d24f4cd..0cd0d2a 100644
> --- a/drivers/video/fbdev/via/via_aux_vt1632.c
> +++ b/drivers/video/fbdev/via/via_aux_vt1632.c
> @@ -35,10 +35,10 @@ static void probe(struct via_aux_bus *bus, u8 addr)
>                 .addr   =       addr,
>                 .name   =       name};
>         /* check vendor id and device id */
> -       const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
> -       u8 tmp[len];
> +       const u8 id[4] = {0x06, 0x11, 0x92, 0x31};
> +       u8 tmp[4];
>
> -       if (!via_aux_read(&drv, 0x00, tmp, len) || memcmp(id, tmp, len))
> +       if (!via_aux_read(&drv, 0x00, tmp, 4) || memcmp(id, tmp, 4))

Generally, hard coding a bunch of numbers like that makes for
confusing and fragile code.

A lot simpler and more obvious solution is like the following.
It silences the compiler warning (-Wvla - pedantic) you while keeping
the original clarity.

        const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
-       u8 tmp[len];
+       u8 tmp[ARRAY_SIZE(id)]; // Using len causes a Wvla warning

HTH
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage
@ 2018-03-08 11:58   ` Emil Velikov
  0 siblings, 0 replies; 7+ messages in thread
From: Emil Velikov @ 2018-03-08 11:58 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Florian Tobias Schandinat, Bartlomiej Zolnierkiewicz, linux-fbdev,
	Gustavo A. R. Silva, Linux-Kernel@Vger. Kernel. Org, ML dri-devel

Hi Gustavo,

On 7 March 2018 at 19:54, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with a fixed-length array instead. Also, remove variable 'len'.
>
What is the reason behind adding -Wvla? Is there a thread some that I
can read up on?

> Notice that no new IDs have been added in seven years.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/video/fbdev/via/via_aux_vt1632.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/via/via_aux_vt1632.c b/drivers/video/fbdev/via/via_aux_vt1632.c
> index d24f4cd..0cd0d2a 100644
> --- a/drivers/video/fbdev/via/via_aux_vt1632.c
> +++ b/drivers/video/fbdev/via/via_aux_vt1632.c
> @@ -35,10 +35,10 @@ static void probe(struct via_aux_bus *bus, u8 addr)
>                 .addr   =       addr,
>                 .name   =       name};
>         /* check vendor id and device id */
> -       const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
> -       u8 tmp[len];
> +       const u8 id[4] = {0x06, 0x11, 0x92, 0x31};
> +       u8 tmp[4];
>
> -       if (!via_aux_read(&drv, 0x00, tmp, len) || memcmp(id, tmp, len))
> +       if (!via_aux_read(&drv, 0x00, tmp, 4) || memcmp(id, tmp, 4))

Generally, hard coding a bunch of numbers like that makes for
confusing and fragile code.

A lot simpler and more obvious solution is like the following.
It silences the compiler warning (-Wvla - pedantic) you while keeping
the original clarity.

        const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
-       u8 tmp[len];
+       u8 tmp[ARRAY_SIZE(id)]; // Using len causes a Wvla warning

HTH
Emil

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage
  2018-03-08 11:58   ` Emil Velikov
@ 2018-03-08 17:27     ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-08 17:27 UTC (permalink / raw)
  To: Emil Velikov, Gustavo A. R. Silva
  Cc: Florian Tobias Schandinat, Bartlomiej Zolnierkiewicz, linux-fbdev,
	Linux-Kernel@Vger. Kernel. Org, ML dri-devel

Hi Emil,

On 03/08/2018 05:58 AM, Emil Velikov wrote:
> Hi Gustavo,
> 
> On 7 March 2018 at 19:54, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote:
>> In preparation to enabling -Wvla, remove VLA and replace it
>> with a fixed-length array instead. Also, remove variable 'len'.
>>
> What is the reason behind adding -Wvla? Is there a thread some that I
> can read up on?
> 

Sure. This is the thread: https://lkml.org/lkml/2018/3/7/621

>> Notice that no new IDs have been added in seven years.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>   drivers/video/fbdev/via/via_aux_vt1632.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/via/via_aux_vt1632.c b/drivers/video/fbdev/via/via_aux_vt1632.c
>> index d24f4cd..0cd0d2a 100644
>> --- a/drivers/video/fbdev/via/via_aux_vt1632.c
>> +++ b/drivers/video/fbdev/via/via_aux_vt1632.c
>> @@ -35,10 +35,10 @@ static void probe(struct via_aux_bus *bus, u8 addr)
>>                  .addr   =       addr,
>>                  .name   =       name};
>>          /* check vendor id and device id */
>> -       const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
>> -       u8 tmp[len];
>> +       const u8 id[4] = {0x06, 0x11, 0x92, 0x31};
>> +       u8 tmp[4];
>>
>> -       if (!via_aux_read(&drv, 0x00, tmp, len) || memcmp(id, tmp, len))
>> +       if (!via_aux_read(&drv, 0x00, tmp, 4) || memcmp(id, tmp, 4))
> 
> Generally, hard coding a bunch of numbers like that makes for
> confusing and fragile code.
> 

You are correct.

> A lot simpler and more obvious solution is like the following.
> It silences the compiler warning (-Wvla - pedantic) you while keeping
> the original clarity.
> 
>          const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
> -       u8 tmp[len];
> +       u8 tmp[ARRAY_SIZE(id)]; // Using len causes a Wvla warning
> 

Yeah, this works just fine.

There are a total of four instances of this same issue in other files 
for the VIA component. I'll fix them and send a single patch shortly.

Thanks for the feedback.
I appreciate it.
--
Gustavo

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage
@ 2018-03-08 17:27     ` Gustavo A. R. Silva
  0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-08 17:27 UTC (permalink / raw)
  To: Emil Velikov, Gustavo A. R. Silva
  Cc: Florian Tobias Schandinat, Bartlomiej Zolnierkiewicz, linux-fbdev,
	Linux-Kernel@Vger. Kernel. Org, ML dri-devel

Hi Emil,

On 03/08/2018 05:58 AM, Emil Velikov wrote:
> Hi Gustavo,
> 
> On 7 March 2018 at 19:54, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote:
>> In preparation to enabling -Wvla, remove VLA and replace it
>> with a fixed-length array instead. Also, remove variable 'len'.
>>
> What is the reason behind adding -Wvla? Is there a thread some that I
> can read up on?
> 

Sure. This is the thread: https://lkml.org/lkml/2018/3/7/621

>> Notice that no new IDs have been added in seven years.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>   drivers/video/fbdev/via/via_aux_vt1632.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/via/via_aux_vt1632.c b/drivers/video/fbdev/via/via_aux_vt1632.c
>> index d24f4cd..0cd0d2a 100644
>> --- a/drivers/video/fbdev/via/via_aux_vt1632.c
>> +++ b/drivers/video/fbdev/via/via_aux_vt1632.c
>> @@ -35,10 +35,10 @@ static void probe(struct via_aux_bus *bus, u8 addr)
>>                  .addr   =       addr,
>>                  .name   =       name};
>>          /* check vendor id and device id */
>> -       const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
>> -       u8 tmp[len];
>> +       const u8 id[4] = {0x06, 0x11, 0x92, 0x31};
>> +       u8 tmp[4];
>>
>> -       if (!via_aux_read(&drv, 0x00, tmp, len) || memcmp(id, tmp, len))
>> +       if (!via_aux_read(&drv, 0x00, tmp, 4) || memcmp(id, tmp, 4))
> 
> Generally, hard coding a bunch of numbers like that makes for
> confusing and fragile code.
> 

You are correct.

> A lot simpler and more obvious solution is like the following.
> It silences the compiler warning (-Wvla - pedantic) you while keeping
> the original clarity.
> 
>          const u8 id[] = {0x06, 0x11, 0x92, 0x31}, len = ARRAY_SIZE(id);
> -       u8 tmp[len];
> +       u8 tmp[ARRAY_SIZE(id)]; // Using len causes a Wvla warning
> 

Yeah, this works just fine.

There are a total of four instances of this same issue in other files 
for the VIA component. I'll fix them and send a single patch shortly.

Thanks for the feedback.
I appreciate it.
--
Gustavo

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-08 17:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-07 19:54 [PATCH] video: fbdev: via_aux_vt1632: remove VLA usage Gustavo A. R. Silva
2018-03-07 19:54 ` Gustavo A. R. Silva
2018-03-08 11:58 ` Emil Velikov
2018-03-08 11:58   ` Emil Velikov
2018-03-08 11:58   ` Emil Velikov
2018-03-08 17:27   ` Gustavo A. R. Silva
2018-03-08 17:27     ` 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.