All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] staging: vt6656: Cleanup of the vnt_get_frame_time function
@ 2020-04-04 14:13 ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:13 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: Oscar Carter, Malcolm Priestley, Quentin Deslandes,
	Amir Mahdi Ghorbanian, devel, linux-kernel

This patch series makes a cleanup of the vnt_get_frame_time function.

The first patch removes the define RATE_54M and changes it for the
ARRAY_SIZE macro. This way avoid possibles issues if the size of the
vnt_frame_time array change in the future but not change accordingly the
RATE_54M constant.

The second patch makes use of the define RATE_11M instead of a magic
number.

The third patch remove unnecessary local variable initialization.

Oscar Carter (3):
  staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
  staging: vt6656: Use define instead of magic number for tx_rate
  staging: vt6656: Remove unnecessary local variable initialization

 drivers/staging/vt6656/baseband.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

--
2.20.1


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

* [PATCH 0/3] staging: vt6656: Cleanup of the vnt_get_frame_time function
@ 2020-04-04 14:13 ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:13 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel

This patch series makes a cleanup of the vnt_get_frame_time function.

The first patch removes the define RATE_54M and changes it for the
ARRAY_SIZE macro. This way avoid possibles issues if the size of the
vnt_frame_time array change in the future but not change accordingly the
RATE_54M constant.

The second patch makes use of the define RATE_11M instead of a magic
number.

The third patch remove unnecessary local variable initialization.

Oscar Carter (3):
  staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
  staging: vt6656: Use define instead of magic number for tx_rate
  staging: vt6656: Remove unnecessary local variable initialization

 drivers/staging/vt6656/baseband.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

--
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
  2020-04-04 14:13 ` Oscar Carter
@ 2020-04-04 14:13   ` Oscar Carter
  -1 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:13 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: Oscar Carter, Malcolm Priestley, Quentin Deslandes,
	Amir Mahdi Ghorbanian, devel, linux-kernel

Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
mismatch. In this way, avoid the possibility of a buffer overflow if
this define is changed in the future to a greater value.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..3e4bd637849a 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -136,7 +136,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
 	unsigned int preamble;
 	unsigned int rate = 0;

-	if (tx_rate > RATE_54M)
+	if (tx_rate >= ARRAY_SIZE(vnt_frame_time))
 		return 0;

 	rate = (unsigned int)vnt_frame_time[tx_rate];
--
2.20.1


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

* [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
@ 2020-04-04 14:13   ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:13 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel

Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
mismatch. In this way, avoid the possibility of a buffer overflow if
this define is changed in the future to a greater value.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index a19a563d8bcc..3e4bd637849a 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -136,7 +136,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
 	unsigned int preamble;
 	unsigned int rate = 0;

-	if (tx_rate > RATE_54M)
+	if (tx_rate >= ARRAY_SIZE(vnt_frame_time))
 		return 0;

 	rate = (unsigned int)vnt_frame_time[tx_rate];
--
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
  2020-04-04 14:13 ` Oscar Carter
@ 2020-04-04 14:13   ` Oscar Carter
  -1 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:13 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: Oscar Carter, Malcolm Priestley, Quentin Deslandes,
	Amir Mahdi Ghorbanian, devel, linux-kernel

Use the define RATE_11M present in the file "device.h" instead of the
magic number 3. So the code is more clear.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/baseband.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index 3e4bd637849a..a785f91c1566 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -24,6 +24,7 @@

 #include <linux/bits.h>
 #include <linux/kernel.h>
+#include "device.h"
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,

 	rate = (unsigned int)vnt_frame_time[tx_rate];

-	if (tx_rate <= 3) {
+	if (tx_rate <= RATE_11M) {
 		if (preamble_type == 1)
 			preamble = 96;
 		else
--
2.20.1


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

* [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
@ 2020-04-04 14:13   ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:13 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel

Use the define RATE_11M present in the file "device.h" instead of the
magic number 3. So the code is more clear.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/baseband.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index 3e4bd637849a..a785f91c1566 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -24,6 +24,7 @@

 #include <linux/bits.h>
 #include <linux/kernel.h>
+#include "device.h"
 #include "mac.h"
 #include "baseband.h"
 #include "rf.h"
@@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,

 	rate = (unsigned int)vnt_frame_time[tx_rate];

-	if (tx_rate <= 3) {
+	if (tx_rate <= RATE_11M) {
 		if (preamble_type == 1)
 			preamble = 96;
 		else
--
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization
  2020-04-04 14:13 ` Oscar Carter
@ 2020-04-04 14:14   ` Oscar Carter
  -1 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:14 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: Oscar Carter, Malcolm Priestley, Quentin Deslandes,
	Amir Mahdi Ghorbanian, devel, linux-kernel

Don't initialize the rate variable as it is set a few lines later.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index a785f91c1566..04393ea6287d 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -135,7 +135,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
 {
 	unsigned int frame_time;
 	unsigned int preamble;
-	unsigned int rate = 0;
+	unsigned int rate;

 	if (tx_rate >= ARRAY_SIZE(vnt_frame_time))
 		return 0;
--
2.20.1


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

* [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization
@ 2020-04-04 14:14   ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-04 14:14 UTC (permalink / raw)
  To: Forest Bond, Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel

Don't initialize the rate variable as it is set a few lines later.

Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
---
 drivers/staging/vt6656/baseband.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
index a785f91c1566..04393ea6287d 100644
--- a/drivers/staging/vt6656/baseband.c
+++ b/drivers/staging/vt6656/baseband.c
@@ -135,7 +135,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
 {
 	unsigned int frame_time;
 	unsigned int preamble;
-	unsigned int rate = 0;
+	unsigned int rate;

 	if (tx_rate >= ARRAY_SIZE(vnt_frame_time))
 		return 0;
--
2.20.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
  2020-04-04 14:13   ` Oscar Carter
@ 2020-04-06 11:13     ` Dan Carpenter
  -1 siblings, 0 replies; 24+ messages in thread
From: Dan Carpenter @ 2020-04-06 11:13 UTC (permalink / raw)
  To: Oscar Carter
  Cc: Forest Bond, Greg Kroah-Hartman, devel, Malcolm Priestley, linux-kernel

On Sat, Apr 04, 2020 at 04:13:58PM +0200, Oscar Carter wrote:
> Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
> mismatch. In this way, avoid the possibility of a buffer overflow if
> this define is changed in the future to a greater value.
> 

Future proofing is not really a valid reason to change this.  We have to
assume that future programmers are not idiots.

The only valid reason to do this is readability, but I'm not convinced
the new version is more readable.

regards,
dan carpenter


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

* Re: [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
@ 2020-04-06 11:13     ` Dan Carpenter
  0 siblings, 0 replies; 24+ messages in thread
From: Dan Carpenter @ 2020-04-06 11:13 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Greg Kroah-Hartman, Malcolm Priestley, Forest Bond, linux-kernel

On Sat, Apr 04, 2020 at 04:13:58PM +0200, Oscar Carter wrote:
> Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
> mismatch. In this way, avoid the possibility of a buffer overflow if
> this define is changed in the future to a greater value.
> 

Future proofing is not really a valid reason to change this.  We have to
assume that future programmers are not idiots.

The only valid reason to do this is readability, but I'm not convinced
the new version is more readable.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
  2020-04-04 14:13   ` Oscar Carter
@ 2020-04-06 11:16     ` Dan Carpenter
  -1 siblings, 0 replies; 24+ messages in thread
From: Dan Carpenter @ 2020-04-06 11:16 UTC (permalink / raw)
  To: Oscar Carter
  Cc: Forest Bond, Greg Kroah-Hartman, devel, Malcolm Priestley, linux-kernel

On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> Use the define RATE_11M present in the file "device.h" instead of the
> magic number 3. So the code is more clear.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  drivers/staging/vt6656/baseband.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> index 3e4bd637849a..a785f91c1566 100644
> --- a/drivers/staging/vt6656/baseband.c
> +++ b/drivers/staging/vt6656/baseband.c
> @@ -24,6 +24,7 @@
> 
>  #include <linux/bits.h>
>  #include <linux/kernel.h>
> +#include "device.h"
>  #include "mac.h"
>  #include "baseband.h"
>  #include "rf.h"
> @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> 
>  	rate = (unsigned int)vnt_frame_time[tx_rate];
> 
> -	if (tx_rate <= 3) {
> +	if (tx_rate <= RATE_11M) {

This is nice.  And if we don't apply patch 1 then it's even nicer
because then "tx_rate" is treated consistently.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter


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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
@ 2020-04-06 11:16     ` Dan Carpenter
  0 siblings, 0 replies; 24+ messages in thread
From: Dan Carpenter @ 2020-04-06 11:16 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Greg Kroah-Hartman, Malcolm Priestley, Forest Bond, linux-kernel

On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> Use the define RATE_11M present in the file "device.h" instead of the
> magic number 3. So the code is more clear.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  drivers/staging/vt6656/baseband.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> index 3e4bd637849a..a785f91c1566 100644
> --- a/drivers/staging/vt6656/baseband.c
> +++ b/drivers/staging/vt6656/baseband.c
> @@ -24,6 +24,7 @@
> 
>  #include <linux/bits.h>
>  #include <linux/kernel.h>
> +#include "device.h"
>  #include "mac.h"
>  #include "baseband.h"
>  #include "rf.h"
> @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> 
>  	rate = (unsigned int)vnt_frame_time[tx_rate];
> 
> -	if (tx_rate <= 3) {
> +	if (tx_rate <= RATE_11M) {

This is nice.  And if we don't apply patch 1 then it's even nicer
because then "tx_rate" is treated consistently.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization
  2020-04-04 14:14   ` Oscar Carter
@ 2020-04-06 11:17     ` Dan Carpenter
  -1 siblings, 0 replies; 24+ messages in thread
From: Dan Carpenter @ 2020-04-06 11:17 UTC (permalink / raw)
  To: Oscar Carter
  Cc: Forest Bond, Greg Kroah-Hartman, devel, Malcolm Priestley, linux-kernel

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter


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

* Re: [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization
@ 2020-04-06 11:17     ` Dan Carpenter
  0 siblings, 0 replies; 24+ messages in thread
From: Dan Carpenter @ 2020-04-06 11:17 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Greg Kroah-Hartman, Malcolm Priestley, Forest Bond, linux-kernel

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
  2020-04-04 14:13   ` Oscar Carter
@ 2020-04-06 14:22     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-06 14:22 UTC (permalink / raw)
  To: Oscar Carter
  Cc: Forest Bond, Malcolm Priestley, Quentin Deslandes,
	Amir Mahdi Ghorbanian, devel, linux-kernel

On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> Use the define RATE_11M present in the file "device.h" instead of the
> magic number 3. So the code is more clear.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/staging/vt6656/baseband.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> index 3e4bd637849a..a785f91c1566 100644
> --- a/drivers/staging/vt6656/baseband.c
> +++ b/drivers/staging/vt6656/baseband.c
> @@ -24,6 +24,7 @@
> 
>  #include <linux/bits.h>
>  #include <linux/kernel.h>
> +#include "device.h"
>  #include "mac.h"
>  #include "baseband.h"
>  #include "rf.h"
> @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> 
>  	rate = (unsigned int)vnt_frame_time[tx_rate];
> 
> -	if (tx_rate <= 3) {
> +	if (tx_rate <= RATE_11M) {
>  		if (preamble_type == 1)
>  			preamble = 96;
>  		else
> --
> 2.20.1

This doesn't apply to my tree :(


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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
@ 2020-04-06 14:22     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-06 14:22 UTC (permalink / raw)
  To: Oscar Carter; +Cc: devel, Malcolm Priestley, linux-kernel, Forest Bond

On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> Use the define RATE_11M present in the file "device.h" instead of the
> magic number 3. So the code is more clear.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/staging/vt6656/baseband.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> index 3e4bd637849a..a785f91c1566 100644
> --- a/drivers/staging/vt6656/baseband.c
> +++ b/drivers/staging/vt6656/baseband.c
> @@ -24,6 +24,7 @@
> 
>  #include <linux/bits.h>
>  #include <linux/kernel.h>
> +#include "device.h"
>  #include "mac.h"
>  #include "baseband.h"
>  #include "rf.h"
> @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> 
>  	rate = (unsigned int)vnt_frame_time[tx_rate];
> 
> -	if (tx_rate <= 3) {
> +	if (tx_rate <= RATE_11M) {
>  		if (preamble_type == 1)
>  			preamble = 96;
>  		else
> --
> 2.20.1

This doesn't apply to my tree :(

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
  2020-04-06 11:13     ` Dan Carpenter
@ 2020-04-06 16:27       ` Oscar Carter
  -1 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-06 16:27 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Forest Bond, Greg Kroah-Hartman, devel, Malcolm Priestley, linux-kernel

On Mon, Apr 06, 2020 at 02:13:23PM +0300, Dan Carpenter wrote:
> On Sat, Apr 04, 2020 at 04:13:58PM +0200, Oscar Carter wrote:
> > Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
> > mismatch. In this way, avoid the possibility of a buffer overflow if
> > this define is changed in the future to a greater value.
> >
>
> Future proofing is not really a valid reason to change this.

Ok, then I leave it as is.

> We have to assume that future programmers are not idiots.
>
That was not my intention. I'm sorry.

> The only valid reason to do this is readability, but I'm not convinced
> the new version is more readable.
>
Ok.

> regards,
> dan carpenter
>
Thanks,
oscar carter

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

* Re: [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
@ 2020-04-06 16:27       ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-06 16:27 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: devel, Greg Kroah-Hartman, Malcolm Priestley, Forest Bond, linux-kernel

On Mon, Apr 06, 2020 at 02:13:23PM +0300, Dan Carpenter wrote:
> On Sat, Apr 04, 2020 at 04:13:58PM +0200, Oscar Carter wrote:
> > Use ARRAY_SIZE to replace the define RATE_54M so we will never have a
> > mismatch. In this way, avoid the possibility of a buffer overflow if
> > this define is changed in the future to a greater value.
> >
>
> Future proofing is not really a valid reason to change this.

Ok, then I leave it as is.

> We have to assume that future programmers are not idiots.
>
That was not my intention. I'm sorry.

> The only valid reason to do this is readability, but I'm not convinced
> the new version is more readable.
>
Ok.

> regards,
> dan carpenter
>
Thanks,
oscar carter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
  2020-04-06 14:22     ` Greg Kroah-Hartman
@ 2020-04-06 16:38       ` Oscar Carter
  -1 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-06 16:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Forest Bond, Malcolm Priestley, Quentin Deslandes,
	Amir Mahdi Ghorbanian, devel, linux-kernel, Dan Carpenter

On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > Use the define RATE_11M present in the file "device.h" instead of the
> > magic number 3. So the code is more clear.
> >
> > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> >  drivers/staging/vt6656/baseband.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> > index 3e4bd637849a..a785f91c1566 100644
> > --- a/drivers/staging/vt6656/baseband.c
> > +++ b/drivers/staging/vt6656/baseband.c
> > @@ -24,6 +24,7 @@
> >
> >  #include <linux/bits.h>
> >  #include <linux/kernel.h>
> > +#include "device.h"
> >  #include "mac.h"
> >  #include "baseband.h"
> >  #include "rf.h"
> > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> >
> >  	rate = (unsigned int)vnt_frame_time[tx_rate];
> >
> > -	if (tx_rate <= 3) {
> > +	if (tx_rate <= RATE_11M) {
> >  		if (preamble_type == 1)
> >  			preamble = 96;
> >  		else
> > --
> > 2.20.1
>
> This doesn't apply to my tree :(
>
Sorry, but I don't understand what it means. This meant that I need to rebase
this patch against your staging-next branch of your staging tree ? Or it means
something else ?

Thanks,
oscar carter

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
@ 2020-04-06 16:38       ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-06 16:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Malcolm Priestley, linux-kernel, Forest Bond, Dan Carpenter

On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > Use the define RATE_11M present in the file "device.h" instead of the
> > magic number 3. So the code is more clear.
> >
> > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> >  drivers/staging/vt6656/baseband.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> > index 3e4bd637849a..a785f91c1566 100644
> > --- a/drivers/staging/vt6656/baseband.c
> > +++ b/drivers/staging/vt6656/baseband.c
> > @@ -24,6 +24,7 @@
> >
> >  #include <linux/bits.h>
> >  #include <linux/kernel.h>
> > +#include "device.h"
> >  #include "mac.h"
> >  #include "baseband.h"
> >  #include "rf.h"
> > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> >
> >  	rate = (unsigned int)vnt_frame_time[tx_rate];
> >
> > -	if (tx_rate <= 3) {
> > +	if (tx_rate <= RATE_11M) {
> >  		if (preamble_type == 1)
> >  			preamble = 96;
> >  		else
> > --
> > 2.20.1
>
> This doesn't apply to my tree :(
>
Sorry, but I don't understand what it means. This meant that I need to rebase
this patch against your staging-next branch of your staging tree ? Or it means
something else ?

Thanks,
oscar carter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
  2020-04-06 16:38       ` Oscar Carter
@ 2020-04-06 17:58         ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-06 17:58 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Malcolm Priestley, linux-kernel, Forest Bond, Dan Carpenter

On Mon, Apr 06, 2020 at 06:38:36PM +0200, Oscar Carter wrote:
> On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > > Use the define RATE_11M present in the file "device.h" instead of the
> > > magic number 3. So the code is more clear.
> > >
> > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > >  drivers/staging/vt6656/baseband.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> > > index 3e4bd637849a..a785f91c1566 100644
> > > --- a/drivers/staging/vt6656/baseband.c
> > > +++ b/drivers/staging/vt6656/baseband.c
> > > @@ -24,6 +24,7 @@
> > >
> > >  #include <linux/bits.h>
> > >  #include <linux/kernel.h>
> > > +#include "device.h"
> > >  #include "mac.h"
> > >  #include "baseband.h"
> > >  #include "rf.h"
> > > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> > >
> > >  	rate = (unsigned int)vnt_frame_time[tx_rate];
> > >
> > > -	if (tx_rate <= 3) {
> > > +	if (tx_rate <= RATE_11M) {
> > >  		if (preamble_type == 1)
> > >  			preamble = 96;
> > >  		else
> > > --
> > > 2.20.1
> >
> > This doesn't apply to my tree :(
> >
> Sorry, but I don't understand what it means. This meant that I need to rebase
> this patch against your staging-next branch of your staging tree ?

Yes, and 3/3 as well, because I dropped the 1/3 patch here.

thanks,

greg k-h

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
@ 2020-04-06 17:58         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 24+ messages in thread
From: Greg Kroah-Hartman @ 2020-04-06 17:58 UTC (permalink / raw)
  To: Oscar Carter
  Cc: devel, Malcolm Priestley, linux-kernel, Dan Carpenter, Forest Bond

On Mon, Apr 06, 2020 at 06:38:36PM +0200, Oscar Carter wrote:
> On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > > Use the define RATE_11M present in the file "device.h" instead of the
> > > magic number 3. So the code is more clear.
> > >
> > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > >  drivers/staging/vt6656/baseband.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> > > index 3e4bd637849a..a785f91c1566 100644
> > > --- a/drivers/staging/vt6656/baseband.c
> > > +++ b/drivers/staging/vt6656/baseband.c
> > > @@ -24,6 +24,7 @@
> > >
> > >  #include <linux/bits.h>
> > >  #include <linux/kernel.h>
> > > +#include "device.h"
> > >  #include "mac.h"
> > >  #include "baseband.h"
> > >  #include "rf.h"
> > > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> > >
> > >  	rate = (unsigned int)vnt_frame_time[tx_rate];
> > >
> > > -	if (tx_rate <= 3) {
> > > +	if (tx_rate <= RATE_11M) {
> > >  		if (preamble_type == 1)
> > >  			preamble = 96;
> > >  		else
> > > --
> > > 2.20.1
> >
> > This doesn't apply to my tree :(
> >
> Sorry, but I don't understand what it means. This meant that I need to rebase
> this patch against your staging-next branch of your staging tree ?

Yes, and 3/3 as well, because I dropped the 1/3 patch here.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
  2020-04-06 17:58         ` Greg Kroah-Hartman
@ 2020-04-07 15:28           ` Oscar Carter
  -1 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-07 15:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Oscar Carter, devel, Malcolm Priestley, linux-kernel,
	Forest Bond, Dan Carpenter

On Mon, Apr 06, 2020 at 07:58:08PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 06, 2020 at 06:38:36PM +0200, Oscar Carter wrote:
> > On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> > > On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > > > Use the define RATE_11M present in the file "device.h" instead of the
> > > > magic number 3. So the code is more clear.
> > > >
> > > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > >  drivers/staging/vt6656/baseband.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> > > > index 3e4bd637849a..a785f91c1566 100644
> > > > --- a/drivers/staging/vt6656/baseband.c
> > > > +++ b/drivers/staging/vt6656/baseband.c
> > > > @@ -24,6 +24,7 @@
> > > >
> > > >  #include <linux/bits.h>
> > > >  #include <linux/kernel.h>
> > > > +#include "device.h"
> > > >  #include "mac.h"
> > > >  #include "baseband.h"
> > > >  #include "rf.h"
> > > > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> > > >
> > > >  	rate = (unsigned int)vnt_frame_time[tx_rate];
> > > >
> > > > -	if (tx_rate <= 3) {
> > > > +	if (tx_rate <= RATE_11M) {
> > > >  		if (preamble_type == 1)
> > > >  			preamble = 96;
> > > >  		else
> > > > --
> > > > 2.20.1
> > >
> > > This doesn't apply to my tree :(
> > >
> > Sorry, but I don't understand what it means. This meant that I need to rebase
> > this patch against your staging-next branch of your staging tree ?
>
> Yes, and 3/3 as well, because I dropped the 1/3 patch here.
>
Ok, I will create a new patch series version rebased against your staging-next
branch and I will send it.

> thanks,
>
> greg k-h

thanks,
oscar carter

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

* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
@ 2020-04-07 15:28           ` Oscar Carter
  0 siblings, 0 replies; 24+ messages in thread
From: Oscar Carter @ 2020-04-07 15:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Oscar Carter, Malcolm Priestley, linux-kernel,
	Forest Bond, Dan Carpenter

On Mon, Apr 06, 2020 at 07:58:08PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 06, 2020 at 06:38:36PM +0200, Oscar Carter wrote:
> > On Mon, Apr 06, 2020 at 04:22:12PM +0200, Greg Kroah-Hartman wrote:
> > > On Sat, Apr 04, 2020 at 04:13:59PM +0200, Oscar Carter wrote:
> > > > Use the define RATE_11M present in the file "device.h" instead of the
> > > > magic number 3. So the code is more clear.
> > > >
> > > > Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> > > > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > >  drivers/staging/vt6656/baseband.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/vt6656/baseband.c b/drivers/staging/vt6656/baseband.c
> > > > index 3e4bd637849a..a785f91c1566 100644
> > > > --- a/drivers/staging/vt6656/baseband.c
> > > > +++ b/drivers/staging/vt6656/baseband.c
> > > > @@ -24,6 +24,7 @@
> > > >
> > > >  #include <linux/bits.h>
> > > >  #include <linux/kernel.h>
> > > > +#include "device.h"
> > > >  #include "mac.h"
> > > >  #include "baseband.h"
> > > >  #include "rf.h"
> > > > @@ -141,7 +142,7 @@ unsigned int vnt_get_frame_time(u8 preamble_type, u8 pkt_type,
> > > >
> > > >  	rate = (unsigned int)vnt_frame_time[tx_rate];
> > > >
> > > > -	if (tx_rate <= 3) {
> > > > +	if (tx_rate <= RATE_11M) {
> > > >  		if (preamble_type == 1)
> > > >  			preamble = 96;
> > > >  		else
> > > > --
> > > > 2.20.1
> > >
> > > This doesn't apply to my tree :(
> > >
> > Sorry, but I don't understand what it means. This meant that I need to rebase
> > this patch against your staging-next branch of your staging tree ?
>
> Yes, and 3/3 as well, because I dropped the 1/3 patch here.
>
Ok, I will create a new patch series version rebased against your staging-next
branch and I will send it.

> thanks,
>
> greg k-h

thanks,
oscar carter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

end of thread, other threads:[~2020-04-07 15:28 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-04 14:13 [PATCH 0/3] staging: vt6656: Cleanup of the vnt_get_frame_time function Oscar Carter
2020-04-04 14:13 ` Oscar Carter
2020-04-04 14:13 ` [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M Oscar Carter
2020-04-04 14:13   ` Oscar Carter
2020-04-06 11:13   ` Dan Carpenter
2020-04-06 11:13     ` Dan Carpenter
2020-04-06 16:27     ` Oscar Carter
2020-04-06 16:27       ` Oscar Carter
2020-04-04 14:13 ` [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate Oscar Carter
2020-04-04 14:13   ` Oscar Carter
2020-04-06 11:16   ` Dan Carpenter
2020-04-06 11:16     ` Dan Carpenter
2020-04-06 14:22   ` Greg Kroah-Hartman
2020-04-06 14:22     ` Greg Kroah-Hartman
2020-04-06 16:38     ` Oscar Carter
2020-04-06 16:38       ` Oscar Carter
2020-04-06 17:58       ` Greg Kroah-Hartman
2020-04-06 17:58         ` Greg Kroah-Hartman
2020-04-07 15:28         ` Oscar Carter
2020-04-07 15:28           ` Oscar Carter
2020-04-04 14:14 ` [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization Oscar Carter
2020-04-04 14:14   ` Oscar Carter
2020-04-06 11:17   ` Dan Carpenter
2020-04-06 11:17     ` Dan Carpenter

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.