* [PATCH] staging: rtl8723bs: modify struct field to use standard bool type @ 2025-04-02 13:18 Abraham Samuel Adekunle 2025-04-03 5:06 ` Julia Lawall 0 siblings, 1 reply; 7+ messages in thread From: Abraham Samuel Adekunle @ 2025-04-02 13:18 UTC (permalink / raw) To: Greg Kroah-Hartman, Julia Lawall; +Cc: outreachy, linux-staging, linux-kernel The struct field uses the uint values 0 and 1 to represent false and true values respectively. Convert cases to use the bool type instead to conform to Linux coding styles and ensure consistency. reported by Coccinelle: Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> --- drivers/staging/rtl8723bs/core/rtw_ap.c | 2 +- drivers/staging/rtl8723bs/include/sta_info.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c index ed6942e289a5..82f54f769ed1 100644 --- a/drivers/staging/rtl8723bs/core/rtw_ap.c +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c @@ -389,7 +389,7 @@ void update_bmc_sta(struct adapter *padapter) psta->qos_option = 0; psta->htpriv.ht_option = false; - psta->ieee8021x_blocked = 0; + psta->ieee8021x_blocked = false; memset((void *)&psta->sta_stats, 0, sizeof(struct stainfo_stats)); diff --git a/drivers/staging/rtl8723bs/include/sta_info.h b/drivers/staging/rtl8723bs/include/sta_info.h index b3535fed3de7..63343998266a 100644 --- a/drivers/staging/rtl8723bs/include/sta_info.h +++ b/drivers/staging/rtl8723bs/include/sta_info.h @@ -86,7 +86,7 @@ struct sta_info { uint qos_option; u8 hwaddr[ETH_ALEN]; - uint ieee8021x_blocked; /* 0: allowed, 1:blocked */ + bool ieee8021x_blocked; uint dot118021XPrivacy; /* aes, tkip... */ union Keytype dot11tkiptxmickey; union Keytype dot11tkiprxmickey; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: modify struct field to use standard bool type 2025-04-02 13:18 [PATCH] staging: rtl8723bs: modify struct field to use standard bool type Abraham Samuel Adekunle @ 2025-04-03 5:06 ` Julia Lawall 2025-04-03 7:09 ` Samuel Abraham 2025-04-03 9:33 ` Samuel Abraham 0 siblings, 2 replies; 7+ messages in thread From: Julia Lawall @ 2025-04-03 5:06 UTC (permalink / raw) To: Abraham Samuel Adekunle Cc: Greg Kroah-Hartman, outreachy, linux-staging, linux-kernel On Wed, 2 Apr 2025, Abraham Samuel Adekunle wrote: > The struct field uses the uint values 0 and 1 to represent false and > true values respectively. > > Convert cases to use the bool type instead to conform to Linux > coding styles and ensure consistency. This is vague. Ensure consistency with what? You can point out that true or false was already being used elsewhere in the code. > > reported by Coccinelle: > > Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> > --- > drivers/staging/rtl8723bs/core/rtw_ap.c | 2 +- > drivers/staging/rtl8723bs/include/sta_info.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c > index ed6942e289a5..82f54f769ed1 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_ap.c > +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c > @@ -389,7 +389,7 @@ void update_bmc_sta(struct adapter *padapter) > psta->qos_option = 0; > psta->htpriv.ht_option = false; > > - psta->ieee8021x_blocked = 0; > + psta->ieee8021x_blocked = false; > > memset((void *)&psta->sta_stats, 0, sizeof(struct stainfo_stats)); > > diff --git a/drivers/staging/rtl8723bs/include/sta_info.h b/drivers/staging/rtl8723bs/include/sta_info.h > index b3535fed3de7..63343998266a 100644 > --- a/drivers/staging/rtl8723bs/include/sta_info.h > +++ b/drivers/staging/rtl8723bs/include/sta_info.h > @@ -86,7 +86,7 @@ struct sta_info { > uint qos_option; > u8 hwaddr[ETH_ALEN]; > > - uint ieee8021x_blocked; /* 0: allowed, 1:blocked */ > + bool ieee8021x_blocked; This declaration doesn't have the same alignment as the others. You should also check whether this is a structure that is read from the hardware. In that case, it would be a concern if the bool field does not have the same size as the uint one. julia > uint dot118021XPrivacy; /* aes, tkip... */ > union Keytype dot11tkiptxmickey; > union Keytype dot11tkiprxmickey; > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: modify struct field to use standard bool type 2025-04-03 5:06 ` Julia Lawall @ 2025-04-03 7:09 ` Samuel Abraham 2025-04-03 9:33 ` Samuel Abraham 1 sibling, 0 replies; 7+ messages in thread From: Samuel Abraham @ 2025-04-03 7:09 UTC (permalink / raw) To: Julia Lawall; +Cc: Greg Kroah-Hartman, outreachy, linux-staging, linux-kernel On Thu, Apr 3, 2025 at 6:06 AM Julia Lawall <julia.lawall@inria.fr> wrote: > > > > On Wed, 2 Apr 2025, Abraham Samuel Adekunle wrote: > > > The struct field uses the uint values 0 and 1 to represent false and > > true values respectively. > > > > Convert cases to use the bool type instead to conform to Linux > > coding styles and ensure consistency. > > This is vague. Ensure consistency with what? You can point out that true > or false was already being used elsewhere in the code. Okay Noted. > > > > > reported by Coccinelle: > > > > Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> > > --- > > drivers/staging/rtl8723bs/core/rtw_ap.c | 2 +- > > drivers/staging/rtl8723bs/include/sta_info.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c > > index ed6942e289a5..82f54f769ed1 100644 > > --- a/drivers/staging/rtl8723bs/core/rtw_ap.c > > +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c > > @@ -389,7 +389,7 @@ void update_bmc_sta(struct adapter *padapter) > > psta->qos_option = 0; > > psta->htpriv.ht_option = false; > > > > - psta->ieee8021x_blocked = 0; > > + psta->ieee8021x_blocked = false; > > > > memset((void *)&psta->sta_stats, 0, sizeof(struct stainfo_stats)); > > > > diff --git a/drivers/staging/rtl8723bs/include/sta_info.h b/drivers/staging/rtl8723bs/include/sta_info.h > > index b3535fed3de7..63343998266a 100644 > > --- a/drivers/staging/rtl8723bs/include/sta_info.h > > +++ b/drivers/staging/rtl8723bs/include/sta_info.h > > @@ -86,7 +86,7 @@ struct sta_info { > > uint qos_option; > > u8 hwaddr[ETH_ALEN]; > > > > - uint ieee8021x_blocked; /* 0: allowed, 1:blocked */ > > + bool ieee8021x_blocked; > > This declaration doesn't have the same alignment as the others. > > You should also check whether this is a structure that is read from the > hardware. In that case, it would be a concern if the bool field does not > have the same size as the uint one. > > julia Thank you very much. I will confirm and effect the changes Adekunle ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: modify struct field to use standard bool type 2025-04-03 5:06 ` Julia Lawall 2025-04-03 7:09 ` Samuel Abraham @ 2025-04-03 9:33 ` Samuel Abraham 2025-04-03 13:54 ` Greg Kroah-Hartman 1 sibling, 1 reply; 7+ messages in thread From: Samuel Abraham @ 2025-04-03 9:33 UTC (permalink / raw) To: Julia Lawall; +Cc: Greg Kroah-Hartman, outreachy, linux-staging, linux-kernel On Thu, Apr 3, 2025 at 6:06 AM Julia Lawall <julia.lawall@inria.fr> wrote: > > > > On Wed, 2 Apr 2025, Abraham Samuel Adekunle wrote: > > > The struct field uses the uint values 0 and 1 to represent false and > > true values respectively. > > > > Convert cases to use the bool type instead to conform to Linux > > coding styles and ensure consistency. > > This is vague. Ensure consistency with what? You can point out that true > or false was already being used elsewhere in the code. > > > > > reported by Coccinelle: > > > > Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> > > --- > > drivers/staging/rtl8723bs/core/rtw_ap.c | 2 +- > > drivers/staging/rtl8723bs/include/sta_info.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c > > index ed6942e289a5..82f54f769ed1 100644 > > --- a/drivers/staging/rtl8723bs/core/rtw_ap.c > > +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c > > @@ -389,7 +389,7 @@ void update_bmc_sta(struct adapter *padapter) > > psta->qos_option = 0; > > psta->htpriv.ht_option = false; > > > > - psta->ieee8021x_blocked = 0; > > + psta->ieee8021x_blocked = false; > > > > memset((void *)&psta->sta_stats, 0, sizeof(struct stainfo_stats)); > > > > diff --git a/drivers/staging/rtl8723bs/include/sta_info.h b/drivers/staging/rtl8723bs/include/sta_info.h > > index b3535fed3de7..63343998266a 100644 > > --- a/drivers/staging/rtl8723bs/include/sta_info.h > > +++ b/drivers/staging/rtl8723bs/include/sta_info.h > > @@ -86,7 +86,7 @@ struct sta_info { > > uint qos_option; > > u8 hwaddr[ETH_ALEN]; > > > > - uint ieee8021x_blocked; /* 0: allowed, 1:blocked */ > > + bool ieee8021x_blocked; > You should also check whether this is a structure that is read from the > hardware. In that case, it would be a concern if the bool field does not > have the same size as the uint one. Hello Julia So following the conversation here, https://lore.kernel.org/outreachy/bf8994cc-b812-f628-ff43-5dae8426e266@inria.fr/T/#u I was able to compare the assembly code of the file before and after my patch and this were my findings Original assembly code for # drivers/staging/rtl8723bs/core/rtw_ap.c:392 psta->ieee8021x_blocked = 0; movl $0, 436(%r12) #, psta->ieee8021x_blocked Assembly Code After Patch # drivers/staging/rtl8723bs/core/rtw_ap.c:392 psta->ieee8021x_blocked = false; movb $0, 434(%r12) #, psta->ieee8021x_blocked Adekunle ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: modify struct field to use standard bool type 2025-04-03 9:33 ` Samuel Abraham @ 2025-04-03 13:54 ` Greg Kroah-Hartman 2025-04-03 14:02 ` Dan Carpenter 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2025-04-03 13:54 UTC (permalink / raw) To: Samuel Abraham; +Cc: Julia Lawall, outreachy, linux-staging, linux-kernel On Thu, Apr 03, 2025 at 10:33:49AM +0100, Samuel Abraham wrote: > On Thu, Apr 3, 2025 at 6:06 AM Julia Lawall <julia.lawall@inria.fr> wrote: > > > > > > > > On Wed, 2 Apr 2025, Abraham Samuel Adekunle wrote: > > > > > The struct field uses the uint values 0 and 1 to represent false and > > > true values respectively. > > > > > > Convert cases to use the bool type instead to conform to Linux > > > coding styles and ensure consistency. > > > > This is vague. Ensure consistency with what? You can point out that true > > or false was already being used elsewhere in the code. > > > > > > > > reported by Coccinelle: > > > > > > Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> > > > --- > > > drivers/staging/rtl8723bs/core/rtw_ap.c | 2 +- > > > drivers/staging/rtl8723bs/include/sta_info.h | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c > > > index ed6942e289a5..82f54f769ed1 100644 > > > --- a/drivers/staging/rtl8723bs/core/rtw_ap.c > > > +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c > > > @@ -389,7 +389,7 @@ void update_bmc_sta(struct adapter *padapter) > > > psta->qos_option = 0; > > > psta->htpriv.ht_option = false; > > > > > > - psta->ieee8021x_blocked = 0; > > > + psta->ieee8021x_blocked = false; > > > > > > memset((void *)&psta->sta_stats, 0, sizeof(struct stainfo_stats)); > > > > > > diff --git a/drivers/staging/rtl8723bs/include/sta_info.h b/drivers/staging/rtl8723bs/include/sta_info.h > > > index b3535fed3de7..63343998266a 100644 > > > --- a/drivers/staging/rtl8723bs/include/sta_info.h > > > +++ b/drivers/staging/rtl8723bs/include/sta_info.h > > > @@ -86,7 +86,7 @@ struct sta_info { > > > uint qos_option; > > > u8 hwaddr[ETH_ALEN]; > > > > > > - uint ieee8021x_blocked; /* 0: allowed, 1:blocked */ > > > + bool ieee8021x_blocked; > > > You should also check whether this is a structure that is read from the > > hardware. In that case, it would be a concern if the bool field does not > > have the same size as the uint one. > Hello Julia > So following the conversation here, > https://lore.kernel.org/outreachy/bf8994cc-b812-f628-ff43-5dae8426e266@inria.fr/T/#u > I was able to compare the assembly code of the file before and after > my patch and this were my findings > > Original assembly code for > # drivers/staging/rtl8723bs/core/rtw_ap.c:392 psta->ieee8021x_blocked = 0; > movl $0, 436(%r12) #, psta->ieee8021x_blocked > > Assembly Code After Patch > # drivers/staging/rtl8723bs/core/rtw_ap.c:392 > psta->ieee8021x_blocked = false; > movb $0, 434(%r12) #, psta->ieee8021x_blocked So the structure size changed? That's not good at all, and is what I was worried about :( Also, the tool 'pahole' might help out here to verify what exactly changed, if you want to dig in further here. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: modify struct field to use standard bool type 2025-04-03 13:54 ` Greg Kroah-Hartman @ 2025-04-03 14:02 ` Dan Carpenter 2025-04-03 14:06 ` Greg Kroah-Hartman 0 siblings, 1 reply; 7+ messages in thread From: Dan Carpenter @ 2025-04-03 14:02 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Samuel Abraham, Julia Lawall, outreachy, linux-staging, linux-kernel On Thu, Apr 03, 2025 at 02:54:01PM +0100, Greg Kroah-Hartman wrote: > > > > diff --git a/drivers/staging/rtl8723bs/include/sta_info.h b/drivers/staging/rtl8723bs/include/sta_info.h > > > > index b3535fed3de7..63343998266a 100644 > > > > --- a/drivers/staging/rtl8723bs/include/sta_info.h > > > > +++ b/drivers/staging/rtl8723bs/include/sta_info.h > > > > @@ -86,7 +86,7 @@ struct sta_info { > > > > uint qos_option; > > > > u8 hwaddr[ETH_ALEN]; > > > > > > > > - uint ieee8021x_blocked; /* 0: allowed, 1:blocked */ > > > > + bool ieee8021x_blocked; > > > > > You should also check whether this is a structure that is read from the > > > hardware. In that case, it would be a concern if the bool field does not > > > have the same size as the uint one. > > Hello Julia > > So following the conversation here, > > https://lore.kernel.org/outreachy/bf8994cc-b812-f628-ff43-5dae8426e266@inria.fr/T/#u > > I was able to compare the assembly code of the file before and after > > my patch and this were my findings > > > > Original assembly code for > > # drivers/staging/rtl8723bs/core/rtw_ap.c:392 psta->ieee8021x_blocked = 0; > > movl $0, 436(%r12) #, psta->ieee8021x_blocked > > > > Assembly Code After Patch > > # drivers/staging/rtl8723bs/core/rtw_ap.c:392 > > psta->ieee8021x_blocked = false; > > movb $0, 434(%r12) #, psta->ieee8021x_blocked > > So the structure size changed? That's not good at all, and is what I > was worried about :( > You had complained about a different struct. struct rx_pkt_attrib. It's fine to modify this one. regards, dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: modify struct field to use standard bool type 2025-04-03 14:02 ` Dan Carpenter @ 2025-04-03 14:06 ` Greg Kroah-Hartman 0 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2025-04-03 14:06 UTC (permalink / raw) To: Dan Carpenter Cc: Samuel Abraham, Julia Lawall, outreachy, linux-staging, linux-kernel On Thu, Apr 03, 2025 at 05:02:45PM +0300, Dan Carpenter wrote: > On Thu, Apr 03, 2025 at 02:54:01PM +0100, Greg Kroah-Hartman wrote: > > > > > diff --git a/drivers/staging/rtl8723bs/include/sta_info.h b/drivers/staging/rtl8723bs/include/sta_info.h > > > > > index b3535fed3de7..63343998266a 100644 > > > > > --- a/drivers/staging/rtl8723bs/include/sta_info.h > > > > > +++ b/drivers/staging/rtl8723bs/include/sta_info.h > > > > > @@ -86,7 +86,7 @@ struct sta_info { > > > > > uint qos_option; > > > > > u8 hwaddr[ETH_ALEN]; > > > > > > > > > > - uint ieee8021x_blocked; /* 0: allowed, 1:blocked */ > > > > > + bool ieee8021x_blocked; > > > > > > > You should also check whether this is a structure that is read from the > > > > hardware. In that case, it would be a concern if the bool field does not > > > > have the same size as the uint one. > > > Hello Julia > > > So following the conversation here, > > > https://lore.kernel.org/outreachy/bf8994cc-b812-f628-ff43-5dae8426e266@inria.fr/T/#u > > > I was able to compare the assembly code of the file before and after > > > my patch and this were my findings > > > > > > Original assembly code for > > > # drivers/staging/rtl8723bs/core/rtw_ap.c:392 psta->ieee8021x_blocked = 0; > > > movl $0, 436(%r12) #, psta->ieee8021x_blocked > > > > > > Assembly Code After Patch > > > # drivers/staging/rtl8723bs/core/rtw_ap.c:392 > > > psta->ieee8021x_blocked = false; > > > movb $0, 434(%r12) #, psta->ieee8021x_blocked > > > > So the structure size changed? That's not good at all, and is what I > > was worried about :( > > > > You had complained about a different struct. struct rx_pkt_attrib. It's > fine to modify this one. Argh, sorry, too many different threads right now, my fault... ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-04-03 14:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-02 13:18 [PATCH] staging: rtl8723bs: modify struct field to use standard bool type Abraham Samuel Adekunle 2025-04-03 5:06 ` Julia Lawall 2025-04-03 7:09 ` Samuel Abraham 2025-04-03 9:33 ` Samuel Abraham 2025-04-03 13:54 ` Greg Kroah-Hartman 2025-04-03 14:02 ` Dan Carpenter 2025-04-03 14:06 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox