[PATCH v4 0/6] staging: rtl8712: use list macros

[PATCH v4 0/6] staging: rtl8712: use list macros

[Himadri Pandya]

This patchset
-  Replaces usage of container_of with list_entry
-  Replaces a while loop with list_for_each
-  Removes an unnecessary NULL check 

V2:
- Include new patches for using list_for_each and removing unnecessary
  NULL check

V3:
- Include explanations for the changes in log messages

V4:
- Change log messages drop one patch

Himadri Pandya (6):
  staging: rtl8712: use list_entry in function _r8712_find_network
  staging: rtl8712: use list_for_each in function _r8712_find_network
  staging: rtl8712: use list_entry in function _free_network_queue
  staging: rtl8712: use list_entry in function
    r8712_get_oldest_wlan_network
  staging: rtl8712: use list_entry in function update_scanned_network
  staging: rtl8712: use list_entry in function
    r8712_select_and_join_from_scan

 drivers/staging/rtl8712/rtl871x_mlme.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH v4 1/6] staging: rtl8712: use list_entry in function _r8712_find_network

[Himadri Pandya]

struct wlan_network contains field list of type list_head. Hence,
getting the address of wlan_network using list_entry is equivalent
of using container_of to do the same. And to use list_entry for list
manipulations is a better choice.

Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
Changes in V4:
	- Change log message
Changes in V3:
	- Improve log message
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index c666e038f43e..9bfceaeaa05e 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -142,7 +142,7 @@ static struct wlan_network *_r8712_find_network(struct  __queue *scanned_queue,
 	phead = &scanned_queue->queue;
 	plist = phead->next;
 	while (plist != phead) {
-		pnetwork = container_of(plist, struct wlan_network, list);
+		pnetwork = list_entry(plist, struct wlan_network, list);
 		plist = plist->next;
 		if (!memcmp(addr, pnetwork->network.MacAddress, ETH_ALEN))
 			break;
-- 
2.17.1

[PATCH v4 2/6] staging: rtl8712: use list_for_each in function _r8712_find_network

[Himadri Pandya]

struct queue is of type list_head and member of struct __queue. Hence,
using list_for_each to iterate over queue is a better choice.

Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
Changes in V4:
	- Change log message
Changes in V3:
	- Improve log message
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 9bfceaeaa05e..6912d73e8172 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -141,9 +141,8 @@ static struct wlan_network *_r8712_find_network(struct  __queue *scanned_queue,
 	spin_lock_irqsave(&scanned_queue->lock, irqL);
 	phead = &scanned_queue->queue;
 	plist = phead->next;
-	while (plist != phead) {
+	list_for_each(plist, phead) {
 		pnetwork = list_entry(plist, struct wlan_network, list);
-		plist = plist->next;
 		if (!memcmp(addr, pnetwork->network.MacAddress, ETH_ALEN))
 			break;
 	}
-- 
2.17.1

[PATCH v4 3/6] staging: rtl8712: use list_entry in function _free_network_queue

[Himadri Pandya]

struct wlan_network contains field list of type list_head. Hence,
getting the address of wlan_network using list_entry is equivalent
of using container_of to do the same. And to use list_entry for list
manipulations is a better choice.

Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
Changes in V4:
	- Change log message
Changes in V3:
	- Improve log message
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 6912d73e8172..e0b4b8fbcc0e 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -162,7 +162,7 @@ static void _free_network_queue(struct _adapter *padapter)
 	phead = &scanned_queue->queue;
 	plist = phead->next;
 	while (!end_of_queue_search(phead, plist)) {
-		pnetwork = container_of(plist, struct wlan_network, list);
+		pnetwork = list_entry(plist, struct wlan_network, list);
 		plist = plist->next;
 		_free_network(pmlmepriv, pnetwork);
 	}
-- 
2.17.1

[PATCH v4 4/6] staging: rtl8712: use list_entry in function r8712_get_oldest_wlan_network

[Himadri Pandya]

struct wlan_network contains field list of type list_head. Hence,
getting the address of wlan_network using list_entry is equivalent
of using container_of to do the same. And to use list_entry for list
manipulations is a better choice.

Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
Changes in V4:
	- Change log message
Changes in V3:
	- Improve log message
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index e0b4b8fbcc0e..05bfc2a60eb9 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -289,7 +289,7 @@ struct	wlan_network *r8712_get_oldest_wlan_network(
 	while (1) {
 		if (end_of_queue_search(phead, plist) ==  true)
 			break;
-		pwlan = container_of(plist, struct wlan_network, list);
+		pwlan = list_entry(plist, struct wlan_network, list);
 		if (pwlan->fixed != true) {
 			if (oldest == NULL ||
 			    time_after((unsigned long)oldest->last_scanned,
-- 
2.17.1

[PATCH v4 5/6] staging: rtl8712: use list_entry in function update_scanned_network

[Himadri Pandya]

struct wlan_network contains field list of type list_head. Hence,
getting the address of wlan_network using list_entry is equivalent
of using container_of to do the same. And to use list_entry for list
manipulations is a better choice.

Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
Changes in V4:
	- Change log message
Changes in V3:
	- Improve log message
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 05bfc2a60eb9..1a15c45aa83b 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -370,7 +370,7 @@ static void update_scanned_network(struct _adapter *adapter,
 		if (end_of_queue_search(phead, plist))
 			break;
 
-		pnetwork = container_of(plist, struct wlan_network, list);
+		pnetwork = list_entry(plist, struct wlan_network, list);
 		if (is_same_network(&pnetwork->network, target))
 			break;
 		if ((oldest == ((struct wlan_network *)0)) ||
-- 
2.17.1

[PATCH v4 6/6] staging: rtl8712: use list_entry in function r8712_select_and_join_from_scan

[Himadri Pandya]

struct wlan_network contains field list of type list_head. Hence,
getting the address of wlan_network using list_entry is equivalent
of using container_of to do the same. And to use list_entry for list
manipulations is a better choice.

Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
Changes in V4:
	- Change log message
Changes in V3:
	- Improve log message
---
 drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 1a15c45aa83b..164422b3ac07 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -1117,8 +1117,8 @@ int r8712_select_and_join_from_scan(struct mlme_priv *pmlmepriv)
 			}
 			return _FAIL;
 		}
-		pnetwork = container_of(pmlmepriv->pscanned,
-					struct wlan_network, list);
+		pnetwork = list_entry(pmlmepriv->pscanned,
+				      struct wlan_network, list);
 		if (pnetwork == NULL)
 			return _FAIL;
 		pmlmepriv->pscanned = pmlmepriv->pscanned->next;
-- 
2.17.1

Re: [PATCH v4 1/6] staging: rtl8712: use list_entry in function _r8712_find_network

[Greg KH]

On Tue, Apr 02, 2019 at 01:39:50AM +0530, Himadri Pandya wrote:
> struct wlan_network contains field list of type list_head. Hence,
> getting the address of wlan_network using list_entry is equivalent
> of using container_of to do the same. And to use list_entry for list
> manipulations is a better choice.
> 
> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> ---
> Changes in V4:
> 	- Change log message
> Changes in V3:
> 	- Improve log message
> ---
>  drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index c666e038f43e..9bfceaeaa05e 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -142,7 +142,7 @@ static struct wlan_network *_r8712_find_network(struct  __queue *scanned_queue,
>  	phead = &scanned_queue->queue;
>  	plist = phead->next;
>  	while (plist != phead) {
> -		pnetwork = container_of(plist, struct wlan_network, list);
> +		pnetwork = list_entry(plist, struct wlan_network, list);
>  		plist = plist->next;

If this really is the list structure, then let's use the list structure
correctly.  We should not be touching any ->next fields, nor comparing
to phead, or any of that logic at all.

You should just need to use something like:
	list_for_each_entry()
for this loop, right?

That will save a bunch of lines, and make things a lot more obvious what
is happening.

thanks,

greg k-h

Re: [PATCH v4 2/6] staging: rtl8712: use list_for_each in function _r8712_find_network

[Greg KH]

On Tue, Apr 02, 2019 at 01:39:51AM +0530, Himadri Pandya wrote:
> struct queue is of type list_head and member of struct __queue. Hence,
> using list_for_each to iterate over queue is a better choice.
> 
> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> ---
> Changes in V4:
> 	- Change log message
> Changes in V3:
> 	- Improve log message
> ---
>  drivers/staging/rtl8712/rtl871x_mlme.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index 9bfceaeaa05e..6912d73e8172 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -141,9 +141,8 @@ static struct wlan_network *_r8712_find_network(struct  __queue *scanned_queue,
>  	spin_lock_irqsave(&scanned_queue->lock, irqL);
>  	phead = &scanned_queue->queue;
>  	plist = phead->next;
> -	while (plist != phead) {
> +	list_for_each(plist, phead) {
>  		pnetwork = list_entry(plist, struct wlan_network, list);
> -		plist = plist->next;

list_for_each_entry() please.

Re: [PATCH v4 3/6] staging: rtl8712: use list_entry in function _free_network_queue

[Greg KH]

On Tue, Apr 02, 2019 at 01:39:52AM +0530, Himadri Pandya wrote:
> struct wlan_network contains field list of type list_head. Hence,
> getting the address of wlan_network using list_entry is equivalent
> of using container_of to do the same. And to use list_entry for list
> manipulations is a better choice.
> 
> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> ---
> Changes in V4:
> 	- Change log message
> Changes in V3:
> 	- Improve log message
> ---
>  drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index 6912d73e8172..e0b4b8fbcc0e 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -162,7 +162,7 @@ static void _free_network_queue(struct _adapter *padapter)
>  	phead = &scanned_queue->queue;
>  	plist = phead->next;
>  	while (!end_of_queue_search(phead, plist)) {
> -		pnetwork = container_of(plist, struct wlan_network, list);
> +		pnetwork = list_entry(plist, struct wlan_network, list);

Very odd, I think you can use list_for_each_entry() here too, but be
careful to verify it, I don't know what end_of_queue_search() is trying
to do.

thanks,

greg k-h

Re: [PATCH v4 4/6] staging: rtl8712: use list_entry in function r8712_get_oldest_wlan_network

[Greg KH]

On Tue, Apr 02, 2019 at 01:39:53AM +0530, Himadri Pandya wrote:
> struct wlan_network contains field list of type list_head. Hence,
> getting the address of wlan_network using list_entry is equivalent
> of using container_of to do the same. And to use list_entry for list
> manipulations is a better choice.
> 
> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> ---
> Changes in V4:
> 	- Change log message
> Changes in V3:
> 	- Improve log message
> ---
>  drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index e0b4b8fbcc0e..05bfc2a60eb9 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -289,7 +289,7 @@ struct	wlan_network *r8712_get_oldest_wlan_network(
>  	while (1) {
>  		if (end_of_queue_search(phead, plist) ==  true)
>  			break;
> -		pwlan = container_of(plist, struct wlan_network, list);
> +		pwlan = list_entry(plist, struct wlan_network, list);

same here, list_for_each_entry()?

Re: [PATCH v4 5/6] staging: rtl8712: use list_entry in function update_scanned_network

[Greg KH]

On Tue, Apr 02, 2019 at 01:39:54AM +0530, Himadri Pandya wrote:
> struct wlan_network contains field list of type list_head. Hence,
> getting the address of wlan_network using list_entry is equivalent
> of using container_of to do the same. And to use list_entry for list
> manipulations is a better choice.
> 
> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> ---
> Changes in V4:
> 	- Change log message
> Changes in V3:
> 	- Improve log message
> ---
>  drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index 05bfc2a60eb9..1a15c45aa83b 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -370,7 +370,7 @@ static void update_scanned_network(struct _adapter *adapter,
>  		if (end_of_queue_search(phead, plist))
>  			break;
>  
> -		pnetwork = container_of(plist, struct wlan_network, list);
> +		pnetwork = list_entry(plist, struct wlan_network, list);

And here for this loop?

Re: [PATCH v4 6/6] staging: rtl8712: use list_entry in function r8712_select_and_join_from_scan

[Greg KH]

On Tue, Apr 02, 2019 at 01:39:55AM +0530, Himadri Pandya wrote:
> struct wlan_network contains field list of type list_head. Hence,
> getting the address of wlan_network using list_entry is equivalent
> of using container_of to do the same. And to use list_entry for list
> manipulations is a better choice.
> 
> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> ---
> Changes in V4:
> 	- Change log message
> Changes in V3:
> 	- Improve log message
> ---
>  drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index 1a15c45aa83b..164422b3ac07 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -1117,8 +1117,8 @@ int r8712_select_and_join_from_scan(struct mlme_priv *pmlmepriv)
>  			}
>  			return _FAIL;
>  		}
> -		pnetwork = container_of(pmlmepriv->pscanned,
> -					struct wlan_network, list);
> +		pnetwork = list_entry(pmlmepriv->pscanned,
> +				      struct wlan_network, list);
>  		if (pnetwork == NULL)
>  			return _FAIL;

A list entry can not be null, so this check is very odd.

Re: [Outreachy kernel] Re: [PATCH v4 6/6] staging: rtl8712: use list_entry in function r8712_select_and_join_from_scan

[Julia Lawall]

On Tue, 2 Apr 2019, Greg KH wrote:

> On Tue, Apr 02, 2019 at 01:39:55AM +0530, Himadri Pandya wrote:
> > struct wlan_network contains field list of type list_head. Hence,
> > getting the address of wlan_network using list_entry is equivalent
> > of using container_of to do the same. And to use list_entry for list
> > manipulations is a better choice.
> >
> > Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> > ---
> > Changes in V4:
> > 	- Change log message
> > Changes in V3:
> > 	- Improve log message
> > ---
> >  drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> > index 1a15c45aa83b..164422b3ac07 100644
> > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> > @@ -1117,8 +1117,8 @@ int r8712_select_and_join_from_scan(struct mlme_priv *pmlmepriv)
> >  			}
> >  			return _FAIL;
> >  		}
> > -		pnetwork = container_of(pmlmepriv->pscanned,
> > -					struct wlan_network, list);
> > +		pnetwork = list_entry(pmlmepriv->pscanned,
> > +				      struct wlan_network, list);
> >  		if (pnetwork == NULL)
> >  			return _FAIL;
>
> A list entry can not be null, so this check is very odd.

I guess it could be, if the value is NULL, because the list field is at
the top of the structure.  However, it looks like the list is build
properly, with init_list_head and then list_add_tail, so the NULL test
should not be needed.

Himadri, there are several very simple loops on lists that could be nicely
converted to list_for_each or list_for_each_entry.  Search for
end_of_queue_search.

julia


>
> --
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190402055701.GJ14075%40kroah.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH v4 3/6] staging: rtl8712: use list_entry in function _free_network_queue

[Julia Lawall]

On Tue, 2 Apr 2019, Greg KH wrote:

> On Tue, Apr 02, 2019 at 01:39:52AM +0530, Himadri Pandya wrote:
> > struct wlan_network contains field list of type list_head. Hence,
> > getting the address of wlan_network using list_entry is equivalent
> > of using container_of to do the same. And to use list_entry for list
> > manipulations is a better choice.
> >
> > Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> > ---
> > Changes in V4:
> > 	- Change log message
> > Changes in V3:
> > 	- Improve log message
> > ---
> >  drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> > index 6912d73e8172..e0b4b8fbcc0e 100644
> > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> > @@ -162,7 +162,7 @@ static void _free_network_queue(struct _adapter *padapter)
> >  	phead = &scanned_queue->queue;
> >  	plist = phead->next;
> >  	while (!end_of_queue_search(phead, plist)) {
> > -		pnetwork = container_of(plist, struct wlan_network, list);
> > +		pnetwork = list_entry(plist, struct wlan_network, list);
>
> Very odd, I think you can use list_for_each_entry() here too, but be
> careful to verify it, I don't know what end_of_queue_search() is trying
> to do.

Nothing. It's an equality test on the head.

julia

>
> thanks,
>
> greg k-h
>
> --
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20190402055608.GG14075%40kroah.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH v4 6/6] staging: rtl8712: use list_entry in function r8712_select_and_join_from_scan

[Himadri Pandya]

[-- Attachment #1: Type: text/plain, Size: 3150 bytes --]

On Tue, 2 Apr, 2019, 12:05 PM Julia Lawall, <julia.lawall@lip6.fr> wrote:

>
>
> On Tue, 2 Apr 2019, Greg KH wrote:
>
> > On Tue, Apr 02, 2019 at 01:39:55AM +0530, Himadri Pandya wrote:
> > > struct wlan_network contains field list of type list_head. Hence,
> > > getting the address of wlan_network using list_entry is equivalent
> > > of using container_of to do the same. And to use list_entry for list
> > > manipulations is a better choice.
> > >
> > > Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> > > ---
> > > Changes in V4:
> > >     - Change log message
> > > Changes in V3:
> > >     - Improve log message
> > > ---
> > >  drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
> b/drivers/staging/rtl8712/rtl871x_mlme.c
> > > index 1a15c45aa83b..164422b3ac07 100644
> > > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> > > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> > > @@ -1117,8 +1117,8 @@ int r8712_select_and_join_from_scan(struct
> mlme_priv *pmlmepriv)
> > >                     }
> > >                     return _FAIL;
> > >             }
> > > -           pnetwork = container_of(pmlmepriv->pscanned,
> > > -                                   struct wlan_network, list);
> > > +           pnetwork = list_entry(pmlmepriv->pscanned,
> > > +                                 struct wlan_network, list);
> > >             if (pnetwork == NULL)
> > >                     return _FAIL;
> >
> > A list entry can not be null, so this check is very odd.
>
> I guess it could be, if the value is NULL, because the list field is at
> the top of the structure.  However, it looks like the list is build
> properly, with init_list_head and then list_add_tail, so the NULL test
> should not be needed.
>

Should I remove the NULL test then?


> Himadri, there are several very simple loops on lists that could be nicely
> converted to list_for_each or list_for_each_entry.  Search for
> end_of_queue_search.
>
> julia
>

On it. Thank you.

- Himadri


>
> >
> > --
> > 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 outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/20190402055701.GJ14075%40kroah.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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1904020831350.2664%40hadrien
> .
> For more options, visit https://groups.google.com/d/optout.
>

[-- Attachment #2: Type: text/html, Size: 5413 bytes --]

Re: [PATCH v4 3/6] staging: rtl8712: use list_entry in function _free_network_queue

[Himadri Pandya]

[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]

On Tue, 2 Apr, 2019, 11:26 AM Greg KH, <gregkh@linuxfoundation.org> wrote:

> On Tue, Apr 02, 2019 at 01:39:52AM +0530, Himadri Pandya wrote:
> > struct wlan_network contains field list of type list_head. Hence,
> > getting the address of wlan_network using list_entry is equivalent
> > of using container_of to do the same. And to use list_entry for list
> > manipulations is a better choice.
> >
> > Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> > ---
> > Changes in V4:
> >       - Change log message
> > Changes in V3:
> >       - Improve log message
> > ---
> >  drivers/staging/rtl8712/rtl871x_mlme.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
> b/drivers/staging/rtl8712/rtl871x_mlme.c
> > index 6912d73e8172..e0b4b8fbcc0e 100644
> > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> > @@ -162,7 +162,7 @@ static void _free_network_queue(struct _adapter
> *padapter)
> >       phead = &scanned_queue->queue;
> >       plist = phead->next;
> >       while (!end_of_queue_search(phead, plist)) {
> > -             pnetwork = container_of(plist, struct wlan_network, list);
> > +             pnetwork = list_entry(plist, struct wlan_network, list);
>
> Very odd, I think you can use list_for_each_entry() here too, but be
> careful to verify it, I don't know what end_of_queue_search() is trying
> to do.
>
> thanks,
>
> greg k-h
>

I'll check. Thank you.

- Himadri

>

[-- Attachment #2: Type: text/html, Size: 2361 bytes --]

Re: [Outreachy kernel] Re: [PATCH v4 6/6] staging: rtl8712: use list_entry in function r8712_select_and_join_from_scan

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 4014 bytes --]



On Tue, 2 Apr 2019, Himadri Pandya wrote:

>
>
> On Tue, 2 Apr, 2019, 12:05 PM Julia Lawall, <julia.lawall@lip6.fr> wrote:
>
>
>       On Tue, 2 Apr 2019, Greg KH wrote:
>
>       > On Tue, Apr 02, 2019 at 01:39:55AM +0530, Himadri Pandya
>       wrote:
>       > > struct wlan_network contains field list of type list_head.
>       Hence,
>       > > getting the address of wlan_network using list_entry is
>       equivalent
>       > > of using container_of to do the same. And to use list_entry
>       for list
>       > > manipulations is a better choice.
>       > >
>       > > Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
>       > > ---
>       > > Changes in V4:
>       > >ￜ ￜ ￜ- Change log message
>       > > Changes in V3:
>       > >ￜ ￜ ￜ- Improve log message
>       > > ---
>       > >ￜ drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
>       > >ￜ 1 file changed, 2 insertions(+), 2 deletions(-)
>       > >
>       > > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
>       b/drivers/staging/rtl8712/rtl871x_mlme.c
>       > > index 1a15c45aa83b..164422b3ac07 100644
>       > > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
>       > > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
>       > > @@ -1117,8 +1117,8 @@ int
>       r8712_select_and_join_from_scan(struct mlme_priv *pmlmepriv)
>       > >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ}
>       > >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜreturn _FAIL;
>       > >ￜ ￜ ￜ ￜ ￜ ￜ ￜ}
>       > > -ￜ ￜ ￜ ￜ ￜ ￜpnetwork = container_of(pmlmepriv->pscanned,
>       > > -ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜstruct wlan_network,
>       list);
>       > > +ￜ ￜ ￜ ￜ ￜ ￜpnetwork = list_entry(pmlmepriv->pscanned,
>       > > +ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜstruct wlan_network,
>       list);
>       > >ￜ ￜ ￜ ￜ ￜ ￜ ￜif (pnetwork == NULL)
>       > >ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜ ￜreturn _FAIL;
>       >
>       > A list entry can not be null, so this check is very odd.
>
>       I guess it could be, if the value is NULL, because the list
>       field is at
>       the top of the structure.ￜ However, it looks like the list is
>       build
>       properly, with init_list_head and then list_add_tail, so the
>       NULL test
>       should not be needed.
>
>
> Should I remove the NULL test then?

Yes, but you need to explain in the log message in terms of the code why
removing it is correct.

julia

>
>
>       Himadri, there are several very simple loops on lists that could
>       be nicely
>       converted to list_for_each or list_for_each_entry.ￜ Search for
>       end_of_queue_search.
>
>       julia
>
>
> On it. Thank you.
>
> - Himadri
>
>
>
>       >
>       > --
>       > 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
>       outreachy-kernel+unsubscribe@googlegroups.com.
>       > To post to this group, send email to
>       outreachy-kernel@googlegroups.com.
>       > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20190402055701.GJ14075%4
>       0kroah.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
>       outreachy-kernel+unsubscribe@googlegroups.com.
>       To post to this group, send email to
>       outreachy-kernel@googlegroups.com.
>       To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.19040208
>       31350.2664%40hadrien.
>       For more options, visit https://groups.google.com/d/optout.
>
>
>

Re: [Outreachy kernel] Re: [PATCH v4 6/6] staging: rtl8712: use list_entry in function r8712_select_and_join_from_scan

[Himadri Pandya]

[-- Attachment #1: Type: text/plain, Size: 4679 bytes --]

On Tue, 2 Apr, 2019, 3:28 PM Julia Lawall, <julia.lawall@lip6.fr> wrote:

>
>
> On Tue, 2 Apr 2019, Himadri Pandya wrote:
>
> >
> >
> > On Tue, 2 Apr, 2019, 12:05 PM Julia Lawall, <julia.lawall@lip6.fr>
> wrote:
> >
> >
> >       On Tue, 2 Apr 2019, Greg KH wrote:
> >
> >       > On Tue, Apr 02, 2019 at 01:39:55AM +0530, Himadri Pandya
> >       wrote:
> >       > > struct wlan_network contains field list of type list_head.
> >       Hence,
> >       > > getting the address of wlan_network using list_entry is
> >       equivalent
> >       > > of using container_of to do the same. And to use list_entry
> >       for list
> >       > > manipulations is a better choice.
> >       > >
> >       > > Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
> >       > > ---
> >       > > Changes in V4:
> >       > >     - Change log message
> >       > > Changes in V3:
> >       > >     - Improve log message
> >       > > ---
> >       > >  drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
> >       > >  1 file changed, 2 insertions(+), 2 deletions(-)
> >       > >
> >       > > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
> >       b/drivers/staging/rtl8712/rtl871x_mlme.c
> >       > > index 1a15c45aa83b..164422b3ac07 100644
> >       > > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> >       > > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> >       > > @@ -1117,8 +1117,8 @@ int
> >       r8712_select_and_join_from_scan(struct mlme_priv *pmlmepriv)
> >       > >                     }
> >       > >                     return _FAIL;
> >       > >             }
> >       > > -           pnetwork = container_of(pmlmepriv->pscanned,
> >       > > -                                   struct wlan_network,
> >       list);
> >       > > +           pnetwork = list_entry(pmlmepriv->pscanned,
> >       > > +                                 struct wlan_network,
> >       list);
> >       > >             if (pnetwork == NULL)
> >       > >                     return _FAIL;
> >       >
> >       > A list entry can not be null, so this check is very odd.
> >
> >       I guess it could be, if the value is NULL, because the list
> >       field is at
> >       the top of the structure.  However, it looks like the list is
> >       build
> >       properly, with init_list_head and then list_add_tail, so the
> >       NULL test
> >       should not be needed.
> >
> >
> > Should I remove the NULL test then?
>
> Yes, but you need to explain in the log message in terms of the code why
> removing it is correct.
>
> julia
>

Okay. I'll do that. Thank you.

- Himadri


> >
> >
> >       Himadri, there are several very simple loops on lists that could
> >       be nicely
> >       converted to list_for_each or list_for_each_entry.  Search for
> >       end_of_queue_search.
> >
> >       julia

>
> >
> > On it. Thank you.
> >
> > - Himadri
> >
> >
> >
> >       >
> >       > --
> >       > 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
> >       outreachy-kernel+unsubscribe@googlegroups.com.
> >       > To post to this group, send email to
> >       outreachy-kernel@googlegroups.com.
> >       > To view this discussion on the web visithttps://
> groups.google.com/d/msgid/outreachy-kernel/20190402055701.GJ14075%4
> >       0kroah.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
> >       outreachy-kernel+unsubscribe@googlegroups.com.
> >       To post to this group, send email to
> >       outreachy-kernel@googlegroups.com.
> >       To view this discussion on the web visithttps://
> groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.19040208
> >       31350.2664%40hadrien.
> >       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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.20.1904021157590.3297%40hadrien
> .
> For more options, visit https://groups.google.com/d/optout.
>

[-- Attachment #2: Type: text/html, Size: 8227 bytes --]