Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Andy Shevchenko]

On Tue, May 22, 2018 at 6:30 AM, Yisheng Xie  wrote:

>> But it's up tu Loca.

Shame on me. I meant Luca, of course!
Luca, sorry.

> OK, I will change it if Loca agree your opinion.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Andy Shevchenko]

On Tue, May 22, 2018 at 6:30 AM, Yisheng Xie  wrote:

>> But it's up tu Loca.

Shame on me. I meant Luca, of course!
Luca, sorry.

> OK, I will change it if Loca agree your opinion.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Yisheng Xie]

Hi Andy,

On 2018/5/22 5:43, Andy Shevchenko wrote:
> On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie  wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used intead of open coded variant.
> 
>> int ret, bt_force_ant_mode;
>>
>> -   for (bt_force_ant_mode = 0;
>> -bt_force_ant_mode < ARRAY_SIZE(modes_str);
>> -bt_force_ant_mode++) {
>> -   if (!strcmp(buf, modes_str[bt_force_ant_mode]))
>> -   break;
>> -   }
>> -
>> -   if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
> 
>> +   bt_force_ant_mode = match_string(modes_str,
>> +ARRAY_SIZE(modes_str), buf);
> 
> One line?

hmm, if use ret instead it will no over 80 chars.

> 
>> +   if (bt_force_ant_mode < 0)
>> return -EINVAL;
> 
> I would rather use
> 
> ret = match_string();
> if (ret < 0)
>  return ret;
> 
> bt_force_... = ret;
> 
> But it's up tu Loca.

OK, I will change it if Loca agree your opinion.

Thanks
Yisheng
> 
>>
>> ret = 0;
> 
> 
> 

Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Yisheng Xie]

Hi Andy,

On 2018/5/22 5:43, Andy Shevchenko wrote:
> On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie  wrote:
>> match_string() returns the index of an array for a matching string,
>> which can be used intead of open coded variant.
> 
>> int ret, bt_force_ant_mode;
>>
>> -   for (bt_force_ant_mode = 0;
>> -bt_force_ant_mode < ARRAY_SIZE(modes_str);
>> -bt_force_ant_mode++) {
>> -   if (!strcmp(buf, modes_str[bt_force_ant_mode]))
>> -   break;
>> -   }
>> -
>> -   if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
> 
>> +   bt_force_ant_mode = match_string(modes_str,
>> +ARRAY_SIZE(modes_str), buf);
> 
> One line?

hmm, if use ret instead it will no over 80 chars.

> 
>> +   if (bt_force_ant_mode < 0)
>> return -EINVAL;
> 
> I would rather use
> 
> ret = match_string();
> if (ret < 0)
>  return ret;
> 
> bt_force_... = ret;
> 
> But it's up tu Loca.

OK, I will change it if Loca agree your opinion.

Thanks
Yisheng
> 
>>
>> ret = 0;
> 
> 
> 

Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Andy Shevchenko]

On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie  wrote:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded variant.

> int ret, bt_force_ant_mode;
>
> -   for (bt_force_ant_mode = 0;
> -bt_force_ant_mode < ARRAY_SIZE(modes_str);
> -bt_force_ant_mode++) {
> -   if (!strcmp(buf, modes_str[bt_force_ant_mode]))
> -   break;
> -   }
> -
> -   if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))

> +   bt_force_ant_mode = match_string(modes_str,
> +ARRAY_SIZE(modes_str), buf);

One line?

> +   if (bt_force_ant_mode < 0)
> return -EINVAL;

I would rather use

ret = match_string();
if (ret < 0)
 return ret;

bt_force_... = ret;

But it's up tu Loca.

>
> ret = 0;



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Andy Shevchenko]

On Mon, May 21, 2018 at 2:57 PM, Yisheng Xie  wrote:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded variant.

> int ret, bt_force_ant_mode;
>
> -   for (bt_force_ant_mode = 0;
> -bt_force_ant_mode < ARRAY_SIZE(modes_str);
> -bt_force_ant_mode++) {
> -   if (!strcmp(buf, modes_str[bt_force_ant_mode]))
> -   break;
> -   }
> -
> -   if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))

> +   bt_force_ant_mode = match_string(modes_str,
> +ARRAY_SIZE(modes_str), buf);

One line?

> +   if (bt_force_ant_mode < 0)
> return -EINVAL;

I would rather use

ret = match_string();
if (ret < 0)
 return ret;

bt_force_... = ret;

But it's up tu Loca.

>
> ret = 0;



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Luca Coelho]

On Mon, 2018-05-21 at 19:57 +0800, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded variant.
> 
> Cc: Kalle Valo 
> Cc: Intel Linux Wireless 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: linux-wirel...@vger.kernel.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Yisheng Xie 
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> index 0e6401c..e8249a6 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -671,14 +671,9 @@ static ssize_t iwl_dbgfs_bt_cmd_read(struct file
> *file, char __user *user_buf,
>   };
>   int ret, bt_force_ant_mode;
>  
> - for (bt_force_ant_mode = 0;
> -  bt_force_ant_mode < ARRAY_SIZE(modes_str);
> -  bt_force_ant_mode++) {
> - if (!strcmp(buf, modes_str[bt_force_ant_mode]))
> - break;
> - }
> -
> - if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
> + bt_force_ant_mode = match_string(modes_str,
> +  ARRAY_SIZE(modes_str),
> buf);
> + if (bt_force_ant_mode < 0)
>   return -EINVAL;
>  
>   ret = 0;

Looks fine, I'll push this to our internal tree for review and take a
closer look at what the match_string() function does exactly.

Thanks for the patch.

--
Cheers,
Luca.

Re: [PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Luca Coelho]

On Mon, 2018-05-21 at 19:57 +0800, Yisheng Xie wrote:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded variant.
> 
> Cc: Kalle Valo 
> Cc: Intel Linux Wireless 
> Cc: Johannes Berg 
> Cc: Emmanuel Grumbach 
> Cc: linux-wirel...@vger.kernel.org
> Cc: net...@vger.kernel.org
> Signed-off-by: Yisheng Xie 
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> index 0e6401c..e8249a6 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -671,14 +671,9 @@ static ssize_t iwl_dbgfs_bt_cmd_read(struct file
> *file, char __user *user_buf,
>   };
>   int ret, bt_force_ant_mode;
>  
> - for (bt_force_ant_mode = 0;
> -  bt_force_ant_mode < ARRAY_SIZE(modes_str);
> -  bt_force_ant_mode++) {
> - if (!strcmp(buf, modes_str[bt_force_ant_mode]))
> - break;
> - }
> -
> - if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
> + bt_force_ant_mode = match_string(modes_str,
> +  ARRAY_SIZE(modes_str),
> buf);
> + if (bt_force_ant_mode < 0)
>   return -EINVAL;
>  
>   ret = 0;

Looks fine, I'll push this to our internal tree for review and take a
closer look at what the match_string() function does exactly.

Thanks for the patch.

--
Cheers,
Luca.

[PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used intead of open coded variant.

Cc: Kalle Valo 
Cc: Intel Linux Wireless 
Cc: Johannes Berg 
Cc: Emmanuel Grumbach 
Cc: linux-wirel...@vger.kernel.org
Cc: net...@vger.kernel.org
Signed-off-by: Yisheng Xie 
---
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index 0e6401c..e8249a6 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -671,14 +671,9 @@ static ssize_t iwl_dbgfs_bt_cmd_read(struct file *file, 
char __user *user_buf,
};
int ret, bt_force_ant_mode;
 
-   for (bt_force_ant_mode = 0;
-bt_force_ant_mode < ARRAY_SIZE(modes_str);
-bt_force_ant_mode++) {
-   if (!strcmp(buf, modes_str[bt_force_ant_mode]))
-   break;
-   }
-
-   if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
+   bt_force_ant_mode = match_string(modes_str,
+ARRAY_SIZE(modes_str), buf);
+   if (bt_force_ant_mode < 0)
return -EINVAL;
 
ret = 0;
-- 
1.7.12.4

[PATCH 07/33] iwlwifi: mvm: use match_string() helper

[Yisheng Xie]

match_string() returns the index of an array for a matching string,
which can be used intead of open coded variant.

Cc: Kalle Valo 
Cc: Intel Linux Wireless 
Cc: Johannes Berg 
Cc: Emmanuel Grumbach 
Cc: linux-wirel...@vger.kernel.org
Cc: net...@vger.kernel.org
Signed-off-by: Yisheng Xie 
---
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index 0e6401c..e8249a6 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -671,14 +671,9 @@ static ssize_t iwl_dbgfs_bt_cmd_read(struct file *file, 
char __user *user_buf,
};
int ret, bt_force_ant_mode;
 
-   for (bt_force_ant_mode = 0;
-bt_force_ant_mode < ARRAY_SIZE(modes_str);
-bt_force_ant_mode++) {
-   if (!strcmp(buf, modes_str[bt_force_ant_mode]))
-   break;
-   }
-
-   if (bt_force_ant_mode >= ARRAY_SIZE(modes_str))
+   bt_force_ant_mode = match_string(modes_str,
+ARRAY_SIZE(modes_str), buf);
+   if (bt_force_ant_mode < 0)
return -EINVAL;
 
ret = 0;
-- 
1.7.12.4