2023-09-21 01:38:31

by Christian Lamparter

[permalink] [raw]
Subject: Re: [PATCH v2 wireless-next 2/9] carl9170: remove unnecessary (void*) conversions

On 9/19/23 06:49, Wu Yunchuan wrote:
> No need cast (void *) to (struct ar9170 *), (u8 *) or (void*).

hmm, your mail went into the spam folder. Good thing I checked.

From what I remember: The reason why these casts were added in
carl9170 was because of compiler warnings/complaints.
Current gcc compilers should be OK (given that the kernel-bot
didn't react, or went your Mail to their spam-folder as well?)
but have you checked these older versions?

(In 6.5.0 Documentation/admin-guide/README.rst states that one
should have at least gcc 5.1 - could you run with those and
see if C=2 W=1 passes?)

Regards,
Christian

> Signed-off-by: Wu Yunchuan <[email protected]>
> ---
> drivers/net/wireless/ath/carl9170/usb.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c
> index e4eb666c6eea..c4edf8355941 100644
> --- a/drivers/net/wireless/ath/carl9170/usb.c
> +++ b/drivers/net/wireless/ath/carl9170/usb.c
> @@ -178,7 +178,7 @@ static void carl9170_usb_tx_data_complete(struct urb *urb)
> switch (urb->status) {
> /* everything is fine */
> case 0:
> - carl9170_tx_callback(ar, (void *)urb->context);
> + carl9170_tx_callback(ar, urb->context);
> break;
>
> /* disconnect */
> @@ -369,7 +369,7 @@ void carl9170_usb_handle_tx_err(struct ar9170 *ar)
> struct urb *urb;
>
> while ((urb = usb_get_from_anchor(&ar->tx_err))) {
> - struct sk_buff *skb = (void *)urb->context;
> + struct sk_buff *skb = urb->context;
>
> carl9170_tx_drop(ar, skb);
> carl9170_tx_callback(ar, skb);
> @@ -397,7 +397,7 @@ static void carl9170_usb_tasklet(struct tasklet_struct *t)
>
> static void carl9170_usb_rx_complete(struct urb *urb)
> {
> - struct ar9170 *ar = (struct ar9170 *)urb->context;
> + struct ar9170 *ar = urb->context;
> int err;
>
> if (WARN_ON_ONCE(!ar))
> @@ -559,7 +559,7 @@ static int carl9170_usb_flush(struct ar9170 *ar)
> int ret, err = 0;
>
> while ((urb = usb_get_from_anchor(&ar->tx_wait))) {
> - struct sk_buff *skb = (void *)urb->context;
> + struct sk_buff *skb = urb->context;
> carl9170_tx_drop(ar, skb);
> carl9170_tx_callback(ar, skb);
> usb_free_urb(urb);
> @@ -668,7 +668,7 @@ int carl9170_exec_cmd(struct ar9170 *ar, const enum carl9170_cmd_oids cmd,
> memcpy(ar->cmd.data, payload, plen);
>
> spin_lock_bh(&ar->cmd_lock);
> - ar->readbuf = (u8 *)out;
> + ar->readbuf = out;
> ar->readlen = outlen;
> spin_unlock_bh(&ar->cmd_lock);
>


2023-09-22 01:35:49

by Wu Yunchuan

[permalink] [raw]
Subject: Re: [PATCH v2 wireless-next 2/9] carl9170: remove unnecessary (void*) conversions

Hi, Christian

On 2023/9/21 03:00, Christian Lamparter wrote:

> On 9/19/23 06:49, Wu Yunchuan wrote:
>> No need cast (void *) to (struct ar9170 *), (u8 *) or (void*).
>
> hmm, your mail went into the spam folder. Good thing I checked.
>
Sometimes mail didn't work :(.
> From what I remember: The reason why these casts were added in
> carl9170 was because of compiler warnings/complaints.
> Current gcc compilers should be OK (given that the kernel-bot
> didn't react, or went your Mail to their spam-folder as well?)
> but have you checked these older versions?
>
My gcc version is 10.2.1. And this seems work when gcc version is
5.4(ubuntu 16.04) or 4.84(Ubuntu 14.04).
I can't  compile Linux under Ubuntu 16.04 because of some compatibility
problems.

So I test a simple demo like this:


void *p_void = NULL;
long var_long = 5;
long *p_long = &var_long;
p_void = p_long;

This works fine.

> (In 6.5.0 Documentation/admin-guide/README.rst states that one
> should have at least gcc 5.1 - could you run with those and
> see if C=2 W=1 passes?)
Oh, I want to do this, but I can't compile or install gcc 5.1 in my
computer.
There are some compatibility problems. I hope I can deal this problem
next week.

Wu Yunchan

>
> Regards,
> Christian
>
>> Signed-off-by: Wu Yunchuan <[email protected]>
>> ---
>>   drivers/net/wireless/ath/carl9170/usb.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/carl9170/usb.c
>> b/drivers/net/wireless/ath/carl9170/usb.c
>> index e4eb666c6eea..c4edf8355941 100644
>> --- a/drivers/net/wireless/ath/carl9170/usb.c
>> +++ b/drivers/net/wireless/ath/carl9170/usb.c
>> @@ -178,7 +178,7 @@ static void carl9170_usb_tx_data_complete(struct
>> urb *urb)
>>       switch (urb->status) {
>>       /* everything is fine */
>>       case 0:
>> -        carl9170_tx_callback(ar, (void *)urb->context);
>> +        carl9170_tx_callback(ar, urb->context);
>>           break;
>>         /* disconnect */
>> @@ -369,7 +369,7 @@ void carl9170_usb_handle_tx_err(struct ar9170 *ar)
>>       struct urb *urb;
>>         while ((urb = usb_get_from_anchor(&ar->tx_err))) {
>> -        struct sk_buff *skb = (void *)urb->context;
>> +        struct sk_buff *skb = urb->context;
>>             carl9170_tx_drop(ar, skb);
>>           carl9170_tx_callback(ar, skb);
>> @@ -397,7 +397,7 @@ static void carl9170_usb_tasklet(struct
>> tasklet_struct *t)
>>     static void carl9170_usb_rx_complete(struct urb *urb)
>>   {
>> -    struct ar9170 *ar = (struct ar9170 *)urb->context;
>> +    struct ar9170 *ar = urb->context;
>>       int err;
>>         if (WARN_ON_ONCE(!ar))
>> @@ -559,7 +559,7 @@ static int carl9170_usb_flush(struct ar9170 *ar)
>>       int ret, err = 0;
>>         while ((urb = usb_get_from_anchor(&ar->tx_wait))) {
>> -        struct sk_buff *skb = (void *)urb->context;
>> +        struct sk_buff *skb = urb->context;
>>           carl9170_tx_drop(ar, skb);
>>           carl9170_tx_callback(ar, skb);
>>           usb_free_urb(urb);
>> @@ -668,7 +668,7 @@ int carl9170_exec_cmd(struct ar9170 *ar, const
>> enum carl9170_cmd_oids cmd,
>>           memcpy(ar->cmd.data, payload, plen);
>>         spin_lock_bh(&ar->cmd_lock);
>> -    ar->readbuf = (u8 *)out;
>> +    ar->readbuf = out;
>>       ar->readlen = outlen;
>>       spin_unlock_bh(&ar->cmd_lock);
>