diff mbox series

[v2,3/8] rndis_wlan: Make use of cpu_to_le16_array()

Message ID 20200817184659.58419-3-andriy.shevchenko@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/8] byteorder: Introduce cpu_to_le16_array() and le16_to_cpu_array() | expand

Commit Message

Andy Shevchenko Aug. 17, 2020, 6:46 p.m. UTC 
Since we have a new helper, let's replace open coded variant by it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Jussi Kivilinna <jussi.kivilinna@iki.fi>
Cc: Kalle Valo <kvalo@codeaurora.org>
---
v2: new patch
 drivers/net/wireless/rndis_wlan.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

kernel test robot Aug. 17, 2020, 10:02 p.m. UTC | #1 
Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on jkirsher-next-queue/dev-queue linuxtv-media/master staging/staging-testing v5.9-rc1 next-20200817]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/byteorder-Introduce-cpu_to_le16_array-and-le16_to_cpu_array/20200818-024849
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/wireless/rndis_wlan.c: In function 'rndis_set_config_parameter':
>> drivers/net/wireless/rndis_wlan.c:907:28: error: passing argument 2 of 'cpu_to_le16_array' from incompatible pointer type [-Werror=incompatible-pointer-types]
     907 |  cpu_to_le16_array(unibuf, param, param_len / sizeof(__le16));
         |                            ^~~~~
         |                            |
         |                            char *
   In file included from include/linux/byteorder/little_endian.h:11,
                    from arch/nios2/include/uapi/asm/byteorder.h:21,
                    from include/asm-generic/bitops/le.h:6,
                    from include/asm-generic/bitops.h:36,
                    from ./arch/nios2/include/generated/asm/bitops.h:1,
                    from include/linux/bitops.h:29,
                    from include/linux/kernel.h:12,
                    from include/linux/list.h:9,
                    from include/linux/module.h:12,
                    from drivers/net/wireless/rndis_wlan.c:16:
   include/linux/byteorder/generic.h:159:62: note: expected 'const u16 *' {aka 'const short unsigned int *'} but argument is of type 'char *'
     159 | static inline void cpu_to_le16_array(__le16 *dst, const u16 *src, size_t len)
         |                                                   ~~~~~~~~~~~^~~
   cc1: some warnings being treated as errors

# https://github.com/0day-ci/linux/commit/9ad6b9bbb11c203d62f21fbf7101b8c04aaea958
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andy-Shevchenko/byteorder-Introduce-cpu_to_le16_array-and-le16_to_cpu_array/20200818-024849
git checkout 9ad6b9bbb11c203d62f21fbf7101b8c04aaea958
vim +/cpu_to_le16_array +907 drivers/net/wireless/rndis_wlan.c

   891	
   892		if (value_type == 2)
   893			netdev_dbg(dev->net, "setting config parameter: %s, value: %s\n",
   894				   param, (u8 *)value);
   895		else
   896			netdev_dbg(dev->net, "setting config parameter: %s, value: %d\n",
   897				   param, *(u32 *)value);
   898	
   899		infobuf->name_offs = cpu_to_le32(sizeof(*infobuf));
   900		infobuf->name_length = cpu_to_le32(param_len);
   901		infobuf->type = cpu_to_le32(value_type);
   902		infobuf->value_offs = cpu_to_le32(sizeof(*infobuf) + param_len);
   903		infobuf->value_length = cpu_to_le32(value_len);
   904	
   905		/* simple string to unicode string conversion */
   906		unibuf = (void *)infobuf + sizeof(*infobuf);
 > 907		cpu_to_le16_array(unibuf, param, param_len / sizeof(__le16));
   908	
   909		if (value_type == 2) {
   910			unibuf = (void *)infobuf + sizeof(*infobuf) + param_len;
   911			cpu_to_le16_array(unibuf, value, value_len / sizeof(__le16));
   912		} else {
   913			dst_value = (void *)infobuf + sizeof(*infobuf) + param_len;
   914			*dst_value = cpu_to_le32(*(u32 *)value);
   915		}
   916	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Kalle Valo Aug. 18, 2020, 5:36 a.m. UTC | #2 
+ linux-wireless

Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> Since we have a new helper, let's replace open coded variant by it.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> ---
> v2: new patch
>  drivers/net/wireless/rndis_wlan.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
> index 8852a1832951..06850ee6d6dc 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -859,9 +859,12 @@ static int rndis_set_config_parameter(struct usbnet *dev, char *param,
>  						int value_type, void *value)
>  {
>  	struct ndis_config_param *infobuf;
> -	int value_len, info_len, param_len, ret, i;
> +	int value_len, info_len, param_len, ret;
>  	__le16 *unibuf;
>  	__le32 *dst_value;
> +#ifdef DEBUG
> +	int i;
> +#endif
>  
>  	if (value_type == 0)
>  		value_len = sizeof(__le32);
> @@ -901,13 +904,11 @@ static int rndis_set_config_parameter(struct usbnet *dev, char *param,
>  
>  	/* simple string to unicode string conversion */
>  	unibuf = (void *)infobuf + sizeof(*infobuf);
> -	for (i = 0; i < param_len / sizeof(__le16); i++)
> -		unibuf[i] = cpu_to_le16(param[i]);
> +	cpu_to_le16_array(unibuf, param, param_len / sizeof(__le16));
>  
>  	if (value_type == 2) {
>  		unibuf = (void *)infobuf + sizeof(*infobuf) + param_len;
> -		for (i = 0; i < value_len / sizeof(__le16); i++)
> -			unibuf[i] = cpu_to_le16(((u8 *)value)[i]);
> +		cpu_to_le16_array(unibuf, value, value_len / sizeof(__le16));
>  	} else {
>  		dst_value = (void *)infobuf + sizeof(*infobuf) + param_len;
>  		*dst_value = cpu_to_le32(*(u32 *)value);

I assume this goes via usb tree:

Acked-by: Kalle Valo <kvalo@codeaurora.org>
Jussi Kivilinna Aug. 18, 2020, 5:36 a.m. UTC | #3 
Hello,

On 17.8.2020 21.46, Andy Shevchenko wrote:
> Since we have a new helper, let's replace open coded variant by it.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> ---
> v2: new patch
>  drivers/net/wireless/rndis_wlan.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
> index 8852a1832951..06850ee6d6dc 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -859,9 +859,12 @@ static int rndis_set_config_parameter(struct usbnet *dev, char *param,
>  						int value_type, void *value)
>  {
>  	struct ndis_config_param *infobuf;
> -	int value_len, info_len, param_len, ret, i;
> +	int value_len, info_len, param_len, ret;
>  	__le16 *unibuf;
>  	__le32 *dst_value;
> +#ifdef DEBUG
> +	int i;
> +#endif
>  
>  	if (value_type == 0)
>  		value_len = sizeof(__le32);
> @@ -901,13 +904,11 @@ static int rndis_set_config_parameter(struct usbnet *dev, char *param,
>  
>  	/* simple string to unicode string conversion */
>  	unibuf = (void *)infobuf + sizeof(*infobuf);
> -	for (i = 0; i < param_len / sizeof(__le16); i++)
> -		unibuf[i] = cpu_to_le16(param[i]);
> +	cpu_to_le16_array(unibuf, param, param_len / sizeof(__le16));

This does not look correct, as kernel test robot noticed. 'param' is ASCII string and 'unibuf' wide character string and loop is making simple 8-bit char to 16-bit char conversion.

>  
>  	if (value_type == 2) {
>  		unibuf = (void *)infobuf + sizeof(*infobuf) + param_len;
> -		for (i = 0; i < value_len / sizeof(__le16); i++)
> -			unibuf[i] = cpu_to_le16(((u8 *)value)[i]);
> +		cpu_to_le16_array(unibuf, value, value_len / sizeof(__le16));

Same here.

-Jussi

>  	} else {
>  		dst_value = (void *)infobuf + sizeof(*infobuf) + param_len;
>  		*dst_value = cpu_to_le32(*(u32 *)value);
>
diff mbox series

Patch

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 8852a1832951..06850ee6d6dc 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -859,9 +859,12 @@  static int rndis_set_config_parameter(struct usbnet *dev, char *param,
 						int value_type, void *value)
 {
 	struct ndis_config_param *infobuf;
-	int value_len, info_len, param_len, ret, i;
+	int value_len, info_len, param_len, ret;
 	__le16 *unibuf;
 	__le32 *dst_value;
+#ifdef DEBUG
+	int i;
+#endif
 
 	if (value_type == 0)
 		value_len = sizeof(__le32);
@@ -901,13 +904,11 @@  static int rndis_set_config_parameter(struct usbnet *dev, char *param,
 
 	/* simple string to unicode string conversion */
 	unibuf = (void *)infobuf + sizeof(*infobuf);
-	for (i = 0; i < param_len / sizeof(__le16); i++)
-		unibuf[i] = cpu_to_le16(param[i]);
+	cpu_to_le16_array(unibuf, param, param_len / sizeof(__le16));
 
 	if (value_type == 2) {
 		unibuf = (void *)infobuf + sizeof(*infobuf) + param_len;
-		for (i = 0; i < value_len / sizeof(__le16); i++)
-			unibuf[i] = cpu_to_le16(((u8 *)value)[i]);
+		cpu_to_le16_array(unibuf, value, value_len / sizeof(__le16));
 	} else {
 		dst_value = (void *)infobuf + sizeof(*infobuf) + param_len;
 		*dst_value = cpu_to_le32(*(u32 *)value);