* [PATCH 0/3] staging: vt6656: Cleanup of the vnt_get_frame_time function
@ 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
` (2 more replies)
0 siblings, 3 replies; 12+ 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] 12+ messages in thread
* [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
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-06 11:13 ` Dan Carpenter
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:14 ` [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization Oscar Carter
2 siblings, 1 reply; 12+ 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] 12+ messages in thread
* [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
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 ` [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:16 ` Dan Carpenter
2020-04-06 14:22 ` Greg Kroah-Hartman
2020-04-04 14:14 ` [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization Oscar Carter
2 siblings, 2 replies; 12+ 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] 12+ messages in thread
* [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization
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 ` [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M 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:14 ` Oscar Carter
2020-04-06 11:17 ` Dan Carpenter
2 siblings, 1 reply; 12+ 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] 12+ messages in thread
* Re: [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M
2020-04-04 14:13 ` [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M Oscar Carter
@ 2020-04-06 11:13 ` Dan Carpenter
2020-04-06 16:27 ` Oscar Carter
0 siblings, 1 reply; 12+ 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] 12+ messages in thread
* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
2020-04-04 14:13 ` [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate Oscar Carter
@ 2020-04-06 11:16 ` Dan Carpenter
2020-04-06 14:22 ` Greg Kroah-Hartman
1 sibling, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization
2020-04-04 14:14 ` [PATCH 3/3] staging: vt6656: Remove unnecessary local variable initialization Oscar Carter
@ 2020-04-06 11:17 ` Dan Carpenter
0 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate
2020-04-04 14:13 ` [PATCH 2/3] staging: vt6656: Use define instead of magic number for tx_rate Oscar Carter
2020-04-06 11:16 ` Dan Carpenter
@ 2020-04-06 14:22 ` Greg Kroah-Hartman
2020-04-06 16:38 ` Oscar Carter
1 sibling, 1 reply; 12+ 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] 12+ 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
0 siblings, 0 replies; 12+ 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] 12+ 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
2020-04-06 17:58 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ 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] 12+ 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
2020-04-07 15:28 ` Oscar Carter
0 siblings, 1 reply; 12+ 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] 12+ 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
0 siblings, 0 replies; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2020-04-07 15:28 UTC | newest]
Thread overview: 12+ 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 ` [PATCH 1/3] staging: vt6656: Use ARRAY_SIZE instead of define RATE_54M Oscar Carter
2020-04-06 11:13 ` Dan Carpenter
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-06 11:16 ` Dan Carpenter
2020-04-06 14:22 ` Greg Kroah-Hartman
2020-04-06 16:38 ` Oscar Carter
2020-04-06 17:58 ` Greg Kroah-Hartman
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-06 11:17 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).