[PATCH -next] cifsd: fix WARNING: convert list_for_each to entry variant in smb2pdu.c

[PATCH -next] cifsd: fix WARNING: convert list_for_each to entry variant in smb2pdu.c

[Baokun Li]

convert list_for_each() to list_for_each_entry() where
applicable.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/cifsd/smb2pdu.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/fs/cifsd/smb2pdu.c b/fs/cifsd/smb2pdu.c
index ac15a9287310..22ef1d9eed1b 100644
--- a/fs/cifsd/smb2pdu.c
+++ b/fs/cifsd/smb2pdu.c
@@ -74,10 +74,7 @@ static inline int check_session_id(struct ksmbd_conn *conn, u64 id)
 struct channel *lookup_chann_list(struct ksmbd_session *sess)
 {
 	struct channel *chann;
-	struct list_head *t;
-
-	list_for_each(t, &sess->ksmbd_chann_list) {
-		chann = list_entry(t, struct channel, chann_list);
+	list_for_each_entry(chann, &sess->ksmbd_chann_list, chann_list) {
 		if (chann && chann->conn == sess->conn)
 			return chann;
 	}
@@ -6258,7 +6255,6 @@ int smb2_cancel(struct ksmbd_work *work)
 	struct smb2_hdr *hdr = work->request_buf;
 	struct smb2_hdr *chdr;
 	struct ksmbd_work *cancel_work = NULL;
-	struct list_head *tmp;
 	int canceled = 0;
 	struct list_head *command_list;
 
@@ -6269,9 +6265,8 @@ int smb2_cancel(struct ksmbd_work *work)
 		command_list = &conn->async_requests;
 
 		spin_lock(&conn->request_lock);
-		list_for_each(tmp, command_list) {
-			cancel_work = list_entry(tmp, struct ksmbd_work,
-						 async_request_entry);
+		list_for_each_entry(cancel_work, command_list,
+				    async_request_entry) {
 			chdr = cancel_work->request_buf;
 
 			if (cancel_work->async_id !=
@@ -6290,9 +6285,7 @@ int smb2_cancel(struct ksmbd_work *work)
 		command_list = &conn->requests;
 
 		spin_lock(&conn->request_lock);
-		list_for_each(tmp, command_list) {
-			cancel_work = list_entry(tmp, struct ksmbd_work,
-						 request_entry);
+		list_for_each_entry(cancel_work, command_list, request_entry) {
 			chdr = cancel_work->request_buf;
 
 			if (chdr->MessageId != hdr->MessageId ||

Re: [PATCH -next] cifsd: fix WARNING: convert list_for_each to entry variant in smb2pdu.c

[Dan Carpenter]

On Thu, Jun 17, 2021 at 02:46:53PM +0800, Baokun Li wrote:
> convert list_for_each() to list_for_each_entry() where
> applicable.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
>  fs/cifsd/smb2pdu.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/cifsd/smb2pdu.c b/fs/cifsd/smb2pdu.c
> index ac15a9287310..22ef1d9eed1b 100644
> --- a/fs/cifsd/smb2pdu.c
> +++ b/fs/cifsd/smb2pdu.c
> @@ -74,10 +74,7 @@ static inline int check_session_id(struct ksmbd_conn *conn, u64 id)
>  struct channel *lookup_chann_list(struct ksmbd_session *sess)
>  {
>  	struct channel *chann;
> -	struct list_head *t;
> -
> -	list_for_each(t, &sess->ksmbd_chann_list) {
> -		chann = list_entry(t, struct channel, chann_list);
> +	list_for_each_entry(chann, &sess->ksmbd_chann_list, chann_list) {
>  		if (chann && chann->conn == sess->conn)

"chan" is the list iterator and it can't be NULL.

>  			return chann;
>  	}

regards,
dan carpenter

Re: [PATCH -next] cifsd: fix WARNING: convert list_for_each to entry variant in smb2pdu.c

[libaokun (A)]

I don't know what the difference is between

list_for_each_entry() and list_for_each() for 'struct channel *chann',

but I don't think there's any difference here.

Would you give me more detial about this, please?

Thank you.

Best Regards.


在 2021/6/17 17:26, Dan Carpenter 写道:
> On Thu, Jun 17, 2021 at 02:46:53PM +0800, Baokun Li wrote:
>> convert list_for_each() to list_for_each_entry() where
>> applicable.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Baokun Li <libaokun1@huawei.com>
>> ---
>>   fs/cifsd/smb2pdu.c | 15 ++++-----------
>>   1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/cifsd/smb2pdu.c b/fs/cifsd/smb2pdu.c
>> index ac15a9287310..22ef1d9eed1b 100644
>> --- a/fs/cifsd/smb2pdu.c
>> +++ b/fs/cifsd/smb2pdu.c
>> @@ -74,10 +74,7 @@ static inline int check_session_id(struct ksmbd_conn *conn, u64 id)
>>   struct channel *lookup_chann_list(struct ksmbd_session *sess)
>>   {
>>   	struct channel *chann;
>> -	struct list_head *t;
>> -
>> -	list_for_each(t, &sess->ksmbd_chann_list) {
>> -		chann = list_entry(t, struct channel, chann_list);
>> +	list_for_each_entry(chann, &sess->ksmbd_chann_list, chann_list) {
>>   		if (chann && chann->conn == sess->conn)
> "chan" is the list iterator and it can't be NULL.
>
>>   			return chann;
>>   	}
> regards,
> dan carpenter
>
> .

Re: [PATCH -next] cifsd: fix WARNING: convert list_for_each to entry variant in smb2pdu.c

[Dan Carpenter]

On Fri, Jun 18, 2021 at 09:44:37AM +0800, libaokun (A) wrote:
> I don't know what the difference is between
> 
> list_for_each_entry() and list_for_each() for 'struct channel *chann',
> 
> but I don't think there's any difference here.

Correct.  There is no difference, but Coccinelle is smart enough to
parse list_for_each_entry() and it's not smart enough to parse
list_for_each().

> 
> Would you give me more detial about this, please?

There is a Coccinelle script scripts/coccinelle/iterators/itnull.cocci
which will complain about the NULL check in the new code so this patch
will introduce a new warning.  We may as well remove the unnecessary
NULL check and avoid the warning.

regards,
dan carpenter

Re: [PATCH -next] cifsd: fix WARNING: convert list_for_each to entry variant in smb2pdu.c

[libaokun (A)]

在 2021/6/18 13:14, Dan Carpenter 写道:
> On Fri, Jun 18, 2021 at 09:44:37AM +0800, libaokun (A) wrote:
>> I don't know what the difference is between
>>
>> list_for_each_entry() and list_for_each() for 'struct channel *chann',
>>
>> but I don't think there's any difference here.
> Correct.  There is no difference, but Coccinelle is smart enough to
> parse list_for_each_entry() and it's not smart enough to parse
> list_for_each().
>
>> Would you give me more detial about this, please?
> There is a Coccinelle script scripts/coccinelle/iterators/itnull.cocci
> which will complain about the NULL check in the new code so this patch
> will introduce a new warning.  We may as well remove the unnecessary
> NULL check and avoid the warning.
>
> regards,
> dan carpenter
>
> .

I get your point, but this bug has nothing to do with my patch.

It's  from e2f34481b24d(cifsd: add server-side procedures for SMB3).

Thank you.

-- 
With Best Regards,
Baokun Li

Re: [PATCH -next] cifsd: fix WARNING: convert list_for_each to entry variant in smb2pdu.c

[Dan Carpenter]

On Fri, Jun 18, 2021 at 03:47:12PM +0800, libaokun (A) wrote:
> 在 2021/6/18 13:14, Dan Carpenter 写道:
> > On Fri, Jun 18, 2021 at 09:44:37AM +0800, libaokun (A) wrote:
> > > I don't know what the difference is between
> > > 
> > > list_for_each_entry() and list_for_each() for 'struct channel *chann',
> > > 
> > > but I don't think there's any difference here.
> > Correct.  There is no difference, but Coccinelle is smart enough to
> > parse list_for_each_entry() and it's not smart enough to parse
> > list_for_each().
> > 
> > > Would you give me more detial about this, please?
> > There is a Coccinelle script scripts/coccinelle/iterators/itnull.cocci
> > which will complain about the NULL check in the new code so this patch
> > will introduce a new warning.  We may as well remove the unnecessary
> > NULL check and avoid the warning.
> > 
> > regards,
> > dan carpenter
> > 
> > .
> 
> I get your point, but this bug has nothing to do with my patch.
> 
> It's  from e2f34481b24d(cifsd: add server-side procedures for SMB3).
> 

Fine, but when Hulk Robot reports the new warning and someone fixes it,
I'm going to insist that the Fixes tag points to your patch.  ;)

regards,
dan carpenter