2017-09-30 01:23:10

by Srishti Sharma

[permalink] [raw]
Subject: [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

For variables of the type struct list_head* use list_entry to access
the current list element instead of using container_of.
Done using the following semantic patch by coccinelle.

@r@
identifier e;
struct list_head* l;
@@

<... when != l == NULL
l;
...>

(
e =
-container_of
+list_entry
(
...)
)

Signed-off-by: Srishti Sharma <[email protected]>
---
drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
index 22cf362..f9df4ac 100644
--- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
+++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
@@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
while (phead != plist) {
int i;

- psta = container_of(plist, struct sta_info,
- hash_list);
+ psta = list_entry(plist, struct sta_info,
+ hash_list);
plist = plist->next;

for (i = 0; i < 16; i++) {
@@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
plist = phead->next;

while (!list_empty(phead)) {
- prframe = container_of(plist, struct recv_frame, list);
+ prframe = list_entry(plist, struct recv_frame, list);

plist = plist->next;

@@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
plist = phead->next;

while (phead != plist) {
- psta = container_of(plist, struct sta_info, hash_list);
+ psta = list_entry(plist, struct sta_info, hash_list);

plist = plist->next;

@@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr)
plist = phead->next;

while (phead != plist) {
- psta = container_of(plist, struct sta_info, hash_list);
+ psta = list_entry(plist, struct sta_info, hash_list);

if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
/* if found the matched address */
@@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
phead = get_list_head(pacl_node_q);
plist = phead->next;
while (phead != plist) {
- paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
+ paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
plist = plist->next;

if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
--
2.7.4


2017-09-30 05:05:04

by Julia Lawall

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of



On Sat, 30 Sep 2017, Srishti Sharma wrote:

> For variables of the type struct list_head* use list_entry to access
> the current list element instead of using container_of.
> Done using the following semantic patch by coccinelle.
>
> @r@
> identifier e;
> struct list_head* l;
> @@
>
> <... when != l == NULL
> l;
> ...>

I don't see what is the goal of the above code. The list_head variable is
not going to be in a statement by itself. There is also no need to check
for l being NULL. If it is NULL, the original code is incorrect too.

> (
> e =
> -container_of
> +list_entry
> (
> ...)
> )

Here you don't need the outer ( ). This makes a disjunction with only
one choice. Since there is only one choice, you don't need the
disjunction.

julia

> Signed-off-by: Srishti Sharma <[email protected]>
> ---
> drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> index 22cf362..f9df4ac 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> @@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
> while (phead != plist) {
> int i;
>
> - psta = container_of(plist, struct sta_info,
> - hash_list);
> + psta = list_entry(plist, struct sta_info,
> + hash_list);
> plist = plist->next;
>
> for (i = 0; i < 16; i++) {
> @@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
> plist = phead->next;
>
> while (!list_empty(phead)) {
> - prframe = container_of(plist, struct recv_frame, list);
> + prframe = list_entry(plist, struct recv_frame, list);
>
> plist = plist->next;
>
> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
> plist = phead->next;
>
> while (phead != plist) {
> - psta = container_of(plist, struct sta_info, hash_list);
> + psta = list_entry(plist, struct sta_info, hash_list);
>
> plist = plist->next;
>
> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr)
> plist = phead->next;
>
> while (phead != plist) {
> - psta = container_of(plist, struct sta_info, hash_list);
> + psta = list_entry(plist, struct sta_info, hash_list);
>
> if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
> /* if found the matched address */
> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
> phead = get_list_head(pacl_node_q);
> plist = phead->next;
> while (phead != plist) {
> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
> plist = plist->next;
>
> if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

2017-09-30 06:01:20

by Srishti Sharma

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall <[email protected]> wrote:
>
>
> On Sat, 30 Sep 2017, Srishti Sharma wrote:
>
>> For variables of the type struct list_head* use list_entry to access
>> the current list element instead of using container_of.
>> Done using the following semantic patch by coccinelle.
>>
>> @r@
>> identifier e;
>> struct list_head* l;
>> @@
>>
>> <... when != l == NULL
>> l;
>> ...>
> I don't see what is the goal of the above code. The list_head variable is
> not going to be in a statement by itself. There is also no need to check
> for l being NULL. If it is NULL, the original code is incorrect too.

Since only those container_of are to replaced with list_entry which
have a variable of type list_head* , I wanted to check if it occurs
anywhere before
container_of , which it only does in it's declaration , because it
can't be in any
statement by itself . I think it will be better to write .

@r@
identifier e;
struct list_head* l;
@@

<...
container_of(l,...);
...>

e =
-container_of
+list_entry
(
....)

>> (
>> e =
>> -container_of
>> +list_entry
>> (
>> ...)
>> )
>
> Here you don't need the outer ( ). This makes a disjunction with only
> one choice. Since there is only one choice, you don't need the
> disjunction.

Thanks a lot , for pointing out the errors .

Regards,
Srishti
> julia
>
>> Signed-off-by: Srishti Sharma <[email protected]>
>> ---
>> drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> index 22cf362..f9df4ac 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> @@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
>> while (phead != plist) {
>> int i;
>>
>> - psta = container_of(plist, struct sta_info,
>> - hash_list);
>> + psta = list_entry(plist, struct sta_info,
>> + hash_list);
>> plist = plist->next;
>>
>> for (i = 0; i < 16; i++) {
>> @@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
>> plist = phead->next;
>>
>> while (!list_empty(phead)) {
>> - prframe = container_of(plist, struct recv_frame, list);
>> + prframe = list_entry(plist, struct recv_frame, list);
>>
>> plist = plist->next;
>>
>> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>> plist = phead->next;
>>
>> while (phead != plist) {
>> - psta = container_of(plist, struct sta_info, hash_list);
>> + psta = list_entry(plist, struct sta_info, hash_list);
>>
>> plist = plist->next;
>>
>> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr)
>> plist = phead->next;
>>
>> while (phead != plist) {
>> - psta = container_of(plist, struct sta_info, hash_list);
>> + psta = list_entry(plist, struct sta_info, hash_list);
>>
>> if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
>> /* if found the matched address */
>> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
>> phead = get_list_head(pacl_node_q);
>> plist = phead->next;
>> while (phead != plist) {
>> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
>> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
>> plist = plist->next;
>>
>> if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>> To post to this group, send email to [email protected].
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>

2017-09-30 06:06:45

by Julia Lawall

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of



On Sat, 30 Sep 2017, Srishti Sharma wrote:

> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall <[email protected]> wrote:
> >
> >
> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
> >
> >> For variables of the type struct list_head* use list_entry to access
> >> the current list element instead of using container_of.
> >> Done using the following semantic patch by coccinelle.
> >>
> >> @r@
> >> identifier e;
> >> struct list_head* l;
> >> @@
> >>
> >> <... when != l == NULL
> >> l;
> >> ...>
> > I don't see what is the goal of the above code. The list_head variable is
> > not going to be in a statement by itself. There is also no need to check
> > for l being NULL. If it is NULL, the original code is incorrect too.
>
> Since only those container_of are to replaced with list_entry which
> have a variable of type list_head* , I wanted to check if it occurs
> anywhere before
> container_of ,

Why? If it is a list, then it seems appropriate to access it using
list_head.

> which it only does in it's declaration , because it
> can't be in any
> statement by itself . I think it will be better to write .
>
> @r@
> identifier e;
> struct list_head* l;
> @@
>
> <...
> container_of(l,...);
> ...>

This doesn't ensure that there is a preceding container_of, if that is
what you are trying to do. The problem is that <... P ...> finds 0 or
more occurrences of pattern P, not 1 or more occurrences. 1 or more
occurrences is <+... P ...+>. But it would be simpler if you want an
occurrence of container_of before the thing you are transforming to put

container_of(l,...);
...
e = ...

But this doesn't make sense either, partly because the preceding
container_of is just not needed, and also because a container_of would not
be in a statement by itself. In Coccinelle, when you put a ; after
something is means that the thing is a complete statement, not just the
end half of a statement.

julia

> e =
> -container_of
> +list_entry
> (
> ....)
>
> >> (
> >> e =
> >> -container_of
> >> +list_entry
> >> (
> >> ...)
> >> )
> >
> > Here you don't need the outer ( ). This makes a disjunction with only
> > one choice. Since there is only one choice, you don't need the
> > disjunction.
>
> Thanks a lot , for pointing out the errors .
>
> Regards,
> Srishti
> > julia
> >
> >> Signed-off-by: Srishti Sharma <[email protected]>
> >> ---
> >> drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++++++------
> >> 1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> index 22cf362..f9df4ac 100644
> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> @@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
> >> while (phead != plist) {
> >> int i;
> >>
> >> - psta = container_of(plist, struct sta_info,
> >> - hash_list);
> >> + psta = list_entry(plist, struct sta_info,
> >> + hash_list);
> >> plist = plist->next;
> >>
> >> for (i = 0; i < 16; i++) {
> >> @@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
> >> plist = phead->next;
> >>
> >> while (!list_empty(phead)) {
> >> - prframe = container_of(plist, struct recv_frame, list);
> >> + prframe = list_entry(plist, struct recv_frame, list);
> >>
> >> plist = plist->next;
> >>
> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
> >> plist = phead->next;
> >>
> >> while (phead != plist) {
> >> - psta = container_of(plist, struct sta_info, hash_list);
> >> + psta = list_entry(plist, struct sta_info, hash_list);
> >>
> >> plist = plist->next;
> >>
> >> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr)
> >> plist = phead->next;
> >>
> >> while (phead != plist) {
> >> - psta = container_of(plist, struct sta_info, hash_list);
> >> + psta = list_entry(plist, struct sta_info, hash_list);
> >>
> >> if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
> >> /* if found the matched address */
> >> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
> >> phead = get_list_head(pacl_node_q);
> >> plist = phead->next;
> >> while (phead != plist) {
> >> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
> >> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
> >> plist = plist->next;
> >>
> >> if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> >> To post to this group, send email to [email protected].
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>

2017-09-30 06:17:45

by Srishti Sharma

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

On Sat, Sep 30, 2017 at 11:36 AM, Julia Lawall <[email protected]> wrote:
>
>
> On Sat, 30 Sep 2017, Srishti Sharma wrote:
>
>> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall <[email protected]> wrote:
>> >
>> >
>> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
>> >
>> >> For variables of the type struct list_head* use list_entry to access
>> >> the current list element instead of using container_of.
>> >> Done using the following semantic patch by coccinelle.
>> >>
>> >> @r@
>> >> identifier e;
>> >> struct list_head* l;
>> >> @@
>> >>
>> >> <... when != l == NULL
>> >> l;
>> >> ...>
>> > I don't see what is the goal of the above code. The list_head variable is
>> > not going to be in a statement by itself. There is also no need to check
>> > for l being NULL. If it is NULL, the original code is incorrect too.
>>
>> Since only those container_of are to replaced with list_entry which
>> have a variable of type list_head* , I wanted to check if it occurs
>> anywhere before
>> container_of ,
>
> Why? If it is a list, then it seems appropriate to access it using
> list_head.
>
>> which it only does in it's declaration , because it
>> can't be in any
>> statement by itself . I think it will be better to write .
>>
>> @r@
>> identifier e;
>> struct list_head* l;
>> @@
>>
>> <...
>> container_of(l,...);
>> ...>
>
> This doesn't ensure that there is a preceding container_of, if that is
> what you are trying to do. The problem is that <... P ...> finds 0 or
> more occurrences of pattern P, not 1 or more occurrences. 1 or more
> occurrences is <+... P ...+>. But it would be simpler if you want an
> occurrence of container_of before the thing you are transforming to put
>
> container_of(l,...);
> ...
> e = ...
>
> But this doesn't make sense either, partly because the preceding
> container_of is just not needed, and also because a container_of would not
> be in a statement by itself. In Coccinelle, when you put a ; after
> something is means that the thing is a complete statement, not just the
> end half of a statement.

So , I guess we can simply write.

e =
-container_of
+list_entry
(l,...)

Regards,
Srishti

>
> julia
>
>> e =
>> -container_of
>> +list_entry
>> (
>> ....)
>>
>> >> (
>> >> e =
>> >> -container_of
>> >> +list_entry
>> >> (
>> >> ...)
>> >> )
>> >
>> > Here you don't need the outer ( ). This makes a disjunction with only
>> > one choice. Since there is only one choice, you don't need the
>> > disjunction.
>>
>> Thanks a lot , for pointing out the errors .
>>
>> Regards,
>> Srishti
>> > julia
>> >
>> >> Signed-off-by: Srishti Sharma <[email protected]>
>> >> ---
>> >> drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++++++------
>> >> 1 file changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> index 22cf362..f9df4ac 100644
>> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> @@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
>> >> while (phead != plist) {
>> >> int i;
>> >>
>> >> - psta = container_of(plist, struct sta_info,
>> >> - hash_list);
>> >> + psta = list_entry(plist, struct sta_info,
>> >> + hash_list);
>> >> plist = plist->next;
>> >>
>> >> for (i = 0; i < 16; i++) {
>> >> @@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
>> >> plist = phead->next;
>> >>
>> >> while (!list_empty(phead)) {
>> >> - prframe = container_of(plist, struct recv_frame, list);
>> >> + prframe = list_entry(plist, struct recv_frame, list);
>> >>
>> >> plist = plist->next;
>> >>
>> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>> >> plist = phead->next;
>> >>
>> >> while (phead != plist) {
>> >> - psta = container_of(plist, struct sta_info, hash_list);
>> >> + psta = list_entry(plist, struct sta_info, hash_list);
>> >>
>> >> plist = plist->next;
>> >>
>> >> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr)
>> >> plist = phead->next;
>> >>
>> >> while (phead != plist) {
>> >> - psta = container_of(plist, struct sta_info, hash_list);
>> >> + psta = list_entry(plist, struct sta_info, hash_list);
>> >>
>> >> if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
>> >> /* if found the matched address */
>> >> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
>> >> phead = get_list_head(pacl_node_q);
>> >> plist = phead->next;
>> >> while (phead != plist) {
>> >> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
>> >> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
>> >> plist = plist->next;
>> >>
>> >> if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
>> >> --
>> >> 2.7.4
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>> >> To post to this group, send email to [email protected].
>> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>

2017-09-30 06:22:04

by Julia Lawall

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of



On Sat, 30 Sep 2017, Srishti Sharma wrote:

> On Sat, Sep 30, 2017 at 11:36 AM, Julia Lawall <[email protected]> wrote:
> >
> >
> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
> >
> >> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall <[email protected]> wrote:
> >> >
> >> >
> >> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
> >> >
> >> >> For variables of the type struct list_head* use list_entry to access
> >> >> the current list element instead of using container_of.
> >> >> Done using the following semantic patch by coccinelle.
> >> >>
> >> >> @r@
> >> >> identifier e;
> >> >> struct list_head* l;
> >> >> @@
> >> >>
> >> >> <... when != l == NULL
> >> >> l;
> >> >> ...>
> >> > I don't see what is the goal of the above code. The list_head variable is
> >> > not going to be in a statement by itself. There is also no need to check
> >> > for l being NULL. If it is NULL, the original code is incorrect too.
> >>
> >> Since only those container_of are to replaced with list_entry which
> >> have a variable of type list_head* , I wanted to check if it occurs
> >> anywhere before
> >> container_of ,
> >
> > Why? If it is a list, then it seems appropriate to access it using
> > list_head.
> >
> >> which it only does in it's declaration , because it
> >> can't be in any
> >> statement by itself . I think it will be better to write .
> >>
> >> @r@
> >> identifier e;
> >> struct list_head* l;
> >> @@
> >>
> >> <...
> >> container_of(l,...);
> >> ...>
> >
> > This doesn't ensure that there is a preceding container_of, if that is
> > what you are trying to do. The problem is that <... P ...> finds 0 or
> > more occurrences of pattern P, not 1 or more occurrences. 1 or more
> > occurrences is <+... P ...+>. But it would be simpler if you want an
> > occurrence of container_of before the thing you are transforming to put
> >
> > container_of(l,...);
> > ...
> > e = ...
> >
> > But this doesn't make sense either, partly because the preceding
> > container_of is just not needed, and also because a container_of would not
> > be in a statement by itself. In Coccinelle, when you put a ; after
> > something is means that the thing is a complete statement, not just the
> > end half of a statement.
>
> So , I guess we can simply write.
>
> e =
> -container_of
> +list_entry
> (l,...)

Yes. You don't even need the e =. As long as there is a call to
container_of on a list_head * value, you can change it.

julia

>
> Regards,
> Srishti
>
> >
> > julia
> >
> >> e =
> >> -container_of
> >> +list_entry
> >> (
> >> ....)
> >>
> >> >> (
> >> >> e =
> >> >> -container_of
> >> >> +list_entry
> >> >> (
> >> >> ...)
> >> >> )
> >> >
> >> > Here you don't need the outer ( ). This makes a disjunction with only
> >> > one choice. Since there is only one choice, you don't need the
> >> > disjunction.
> >>
> >> Thanks a lot , for pointing out the errors .
> >>
> >> Regards,
> >> Srishti
> >> > julia
> >> >
> >> >> Signed-off-by: Srishti Sharma <[email protected]>
> >> >> ---
> >> >> drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++++++------
> >> >> 1 file changed, 6 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> >> index 22cf362..f9df4ac 100644
> >> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> >> >> @@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
> >> >> while (phead != plist) {
> >> >> int i;
> >> >>
> >> >> - psta = container_of(plist, struct sta_info,
> >> >> - hash_list);
> >> >> + psta = list_entry(plist, struct sta_info,
> >> >> + hash_list);
> >> >> plist = plist->next;
> >> >>
> >> >> for (i = 0; i < 16; i++) {
> >> >> @@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
> >> >> plist = phead->next;
> >> >>
> >> >> while (!list_empty(phead)) {
> >> >> - prframe = container_of(plist, struct recv_frame, list);
> >> >> + prframe = list_entry(plist, struct recv_frame, list);
> >> >>
> >> >> plist = plist->next;
> >> >>
> >> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
> >> >> plist = phead->next;
> >> >>
> >> >> while (phead != plist) {
> >> >> - psta = container_of(plist, struct sta_info, hash_list);
> >> >> + psta = list_entry(plist, struct sta_info, hash_list);
> >> >>
> >> >> plist = plist->next;
> >> >>
> >> >> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr)
> >> >> plist = phead->next;
> >> >>
> >> >> while (phead != plist) {
> >> >> - psta = container_of(plist, struct sta_info, hash_list);
> >> >> + psta = list_entry(plist, struct sta_info, hash_list);
> >> >>
> >> >> if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
> >> >> /* if found the matched address */
> >> >> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
> >> >> phead = get_list_head(pacl_node_q);
> >> >> plist = phead->next;
> >> >> while (phead != plist) {
> >> >> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
> >> >> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
> >> >> plist = plist->next;
> >> >>
> >> >> if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
> >> >> --
> >> >> 2.7.4
> >> >>
> >> >> --
> >> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> >> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> >> >> To post to this group, send email to [email protected].
> >> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
> >> >> For more options, visit https://groups.google.com/d/optout.
> >> >>
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAB3L5ox9u0pj04pjkiC8MRgx%3DMZqwh%3Dg%3D2VE0-ZG8NYvrUrDBg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

2017-09-30 06:24:12

by Srishti Sharma

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] Staging: rtl8188eu: core: Use list_entry instead of container_of

On Sat, Sep 30, 2017 at 11:51 AM, Julia Lawall <[email protected]> wrote:
>
>
> On Sat, 30 Sep 2017, Srishti Sharma wrote:
>
>> On Sat, Sep 30, 2017 at 11:36 AM, Julia Lawall <[email protected]> wrote:
>> >
>> >
>> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
>> >
>> >> On Sat, Sep 30, 2017 at 10:35 AM, Julia Lawall <[email protected]> wrote:
>> >> >
>> >> >
>> >> > On Sat, 30 Sep 2017, Srishti Sharma wrote:
>> >> >
>> >> >> For variables of the type struct list_head* use list_entry to access
>> >> >> the current list element instead of using container_of.
>> >> >> Done using the following semantic patch by coccinelle.
>> >> >>
>> >> >> @r@
>> >> >> identifier e;
>> >> >> struct list_head* l;
>> >> >> @@
>> >> >>
>> >> >> <... when != l == NULL
>> >> >> l;
>> >> >> ...>
>> >> > I don't see what is the goal of the above code. The list_head variable is
>> >> > not going to be in a statement by itself. There is also no need to check
>> >> > for l being NULL. If it is NULL, the original code is incorrect too.
>> >>
>> >> Since only those container_of are to replaced with list_entry which
>> >> have a variable of type list_head* , I wanted to check if it occurs
>> >> anywhere before
>> >> container_of ,
>> >
>> > Why? If it is a list, then it seems appropriate to access it using
>> > list_head.
>> >
>> >> which it only does in it's declaration , because it
>> >> can't be in any
>> >> statement by itself . I think it will be better to write .
>> >>
>> >> @r@
>> >> identifier e;
>> >> struct list_head* l;
>> >> @@
>> >>
>> >> <...
>> >> container_of(l,...);
>> >> ...>
>> >
>> > This doesn't ensure that there is a preceding container_of, if that is
>> > what you are trying to do. The problem is that <... P ...> finds 0 or
>> > more occurrences of pattern P, not 1 or more occurrences. 1 or more
>> > occurrences is <+... P ...+>. But it would be simpler if you want an
>> > occurrence of container_of before the thing you are transforming to put
>> >
>> > container_of(l,...);
>> > ...
>> > e = ...
>> >
>> > But this doesn't make sense either, partly because the preceding
>> > container_of is just not needed, and also because a container_of would not
>> > be in a statement by itself. In Coccinelle, when you put a ; after
>> > something is means that the thing is a complete statement, not just the
>> > end half of a statement.
>>
>> So , I guess we can simply write.
>>
>> e =
>> -container_of
>> +list_entry
>> (l,...)
>
> Yes. You don't even need the e =. As long as there is a call to
> container_of on a list_head * value, you can change it.

Okay, Thanks again :) , I'll make the changes and resend .

Regards,
Srishti
>
> julia
>
>>
>> Regards,
>> Srishti
>>
>> >
>> > julia
>> >
>> >> e =
>> >> -container_of
>> >> +list_entry
>> >> (
>> >> ....)
>> >>
>> >> >> (
>> >> >> e =
>> >> >> -container_of
>> >> >> +list_entry
>> >> >> (
>> >> >> ...)
>> >> >> )
>> >> >
>> >> > Here you don't need the outer ( ). This makes a disjunction with only
>> >> > one choice. Since there is only one choice, you don't need the
>> >> > disjunction.
>> >>
>> >> Thanks a lot , for pointing out the errors .
>> >>
>> >> Regards,
>> >> Srishti
>> >> > julia
>> >> >
>> >> >> Signed-off-by: Srishti Sharma <[email protected]>
>> >> >> ---
>> >> >> drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 12 ++++++------
>> >> >> 1 file changed, 6 insertions(+), 6 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> >> index 22cf362..f9df4ac 100644
>> >> >> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> >> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
>> >> >> @@ -152,8 +152,8 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
>> >> >> while (phead != plist) {
>> >> >> int i;
>> >> >>
>> >> >> - psta = container_of(plist, struct sta_info,
>> >> >> - hash_list);
>> >> >> + psta = list_entry(plist, struct sta_info,
>> >> >> + hash_list);
>> >> >> plist = plist->next;
>> >> >>
>> >> >> for (i = 0; i < 16; i++) {
>> >> >> @@ -323,7 +323,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
>> >> >> plist = phead->next;
>> >> >>
>> >> >> while (!list_empty(phead)) {
>> >> >> - prframe = container_of(plist, struct recv_frame, list);
>> >> >> + prframe = list_entry(plist, struct recv_frame, list);
>> >> >>
>> >> >> plist = plist->next;
>> >> >>
>> >> >> @@ -399,7 +399,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>> >> >> plist = phead->next;
>> >> >>
>> >> >> while (phead != plist) {
>> >> >> - psta = container_of(plist, struct sta_info, hash_list);
>> >> >> + psta = list_entry(plist, struct sta_info, hash_list);
>> >> >>
>> >> >> plist = plist->next;
>> >> >>
>> >> >> @@ -435,7 +435,7 @@ struct sta_info *rtw_get_stainfo(struct sta_priv *pstapriv, u8 *hwaddr)
>> >> >> plist = phead->next;
>> >> >>
>> >> >> while (phead != plist) {
>> >> >> - psta = container_of(plist, struct sta_info, hash_list);
>> >> >> + psta = list_entry(plist, struct sta_info, hash_list);
>> >> >>
>> >> >> if ((!memcmp(psta->hwaddr, addr, ETH_ALEN)) == true) {
>> >> >> /* if found the matched address */
>> >> >> @@ -493,7 +493,7 @@ u8 rtw_access_ctrl(struct adapter *padapter, u8 *mac_addr)
>> >> >> phead = get_list_head(pacl_node_q);
>> >> >> plist = phead->next;
>> >> >> while (phead != plist) {
>> >> >> - paclnode = container_of(plist, struct rtw_wlan_acl_node, list);
>> >> >> + paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
>> >> >> plist = plist->next;
>> >> >>
>> >> >> if (!memcmp(paclnode->addr, mac_addr, ETH_ALEN)) {
>> >> >> --
>> >> >> 2.7.4
>> >> >>
>> >> >> --
>> >> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> >> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>> >> >> To post to this group, send email to [email protected].
>> >> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1506734581-10932-1-git-send-email-srishtishar%40gmail.com.
>> >> >> For more options, visit https://groups.google.com/d/optout.
>> >> >>
>> >>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>> To post to this group, send email to [email protected].
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAB3L5ox9u0pj04pjkiC8MRgx%3DMZqwh%3Dg%3D2VE0-ZG8NYvrUrDBg%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>