[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    Re: [dpdk-dev] [PATCH] drivers/net/i40e/:remove i40e_nvmupd_command redundant code
From:       "Zhang, Qi Z" <qi.z.zhang () intel ! com>
Date:       2018-09-30 2:12:01
Message-ID: 039ED4275CED7440929022BC67E70611532A833E () SHSMSX103 ! ccr ! corp ! intel ! com
[Download RAW message or body]

Hi Han:

> -----Original Message-----
> From: Li Han [mailto:han.li1@zte.com.cn]
> Sent: Thursday, September 27, 2018 3:27 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Li Han <han.li1@zte.com.cn>
> Subject: [PATCH] [dpdk-dev] drivers/net/i40e/:remove
> i40e_nvmupd_command redundant code
> 
> in function i40e_nvmupd_validate_command, when "cmd->data_size < 1"
> it'll return I40E_NVMUPD_INVALID,and in function "i40e_nvmupd_command"
> when "upd_cmd == I40E_NVMUPD_STATUS",it check "!cmd->data_size" again,
> but it's no need.

Thanks for contribute this.
I have no objection for this change.

But you know , base code is kind of special code in driver, it is shared by different \
platform, Intel DPDk team will update base code timely based on Intel's base code \
team's release. So we are not prefer to make much difference from the original \
version which make things complex for code update. So usually we will only accept \
necessary patch (like critical fix, missing device configure... which may impact our \
customer)

Since this is the patch for code clean, I think it's better we just feedback to our \
base code team , and hope this could be captured in next update. Is that ok for you?
Thanks for your understanding

Regards
Qi
> 
> Signed-off-by: Li Han <han.li1@zte.com.cn>
> ---
> drivers/net/i40e/base/i40e_nvm.c | 5 -----
> 1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/net/i40e/base/i40e_nvm.c
> b/drivers/net/i40e/base/i40e_nvm.c
> index c77dac0..5de22e4 100644
> --- a/drivers/net/i40e/base/i40e_nvm.c
> +++ b/drivers/net/i40e/base/i40e_nvm.c
> @@ -869,11 +869,6 @@ enum i40e_status_code
> i40e_nvmupd_command(struct i40e_hw *hw,
> 	 * going into the state machine
> 	 */
> 	if (upd_cmd == I40E_NVMUPD_STATUS) {
> -		if (!cmd->data_size) {
> -			*perrno = -EFAULT;
> -			return I40E_ERR_BUF_TOO_SHORT;
> -		}
> -
> 		bytes[0] = hw->nvmupd_state;
> 
> 		if (cmd->data_size >= 4) {
> --
> 1.8.3.1


[prev in list] [next in list] [prev in thread] [next in thread]