Message ID | 20220207081522.12111-1-yang.lee@linux.alibaba.com |
---|---|
State | Superseded |
Headers | show |
Series | [-next,v2] scsi: pm8001: clean up some inconsistent indenting | expand |
On 2/7/22 17:15, Yang Li wrote: > Eliminate the follow smatch warning: > drivers/scsi/pm8001/pm8001_ctl.c:760 pm8001_update_flash() warn: > inconsistent indenting > > Reported-by: Abaci Robot <abaci@linux.alibaba.com> > Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> > --- > > Changes in v2: > --According to Damien's suggestion > 1) Start multi-line comments with a line that has only "/*". > 2) Align the conditions together to make this code more readable. > 3) Using a local variable for value which is calculated too many times. > > drivers/scsi/pm8001/pm8001_ctl.c | 61 ++++++++++++++++---------------- > 1 file changed, 31 insertions(+), 30 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c > index 41a63c9b719b..0d9533ba8d27 100644 > --- a/drivers/scsi/pm8001/pm8001_ctl.c > +++ b/drivers/scsi/pm8001/pm8001_ctl.c > @@ -727,6 +727,8 @@ static int pm8001_update_flash(struct pm8001_hba_info *pm8001_ha) > u32 sizeRead = 0; > u32 ret = 0; > u32 length = 1024 * 16 + sizeof(*payload) - 1; > + u32 fc_len = 0; I think that the "= 0" is not needed. > + u8 *read_buf; > > if (pm8001_ha->fw_image->size < 28) { > pm8001_ha->fw_status = FAIL_FILE_SIZE; > @@ -755,36 +757,35 @@ static int pm8001_update_flash(struct pm8001_hba_info *pm8001_ha) > fwControl->retcode = 0;/* OUT */ > fwControl->offset = loopNumber * IOCTL_BUF_SIZE;/*OUT */ > > - /* for the last chunk of data in case file size is not even with > - 4k, load only the rest*/ > - if (((loopcount-loopNumber) == 1) && > - ((partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE)) { > - fwControl->len = > - (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE; > - memcpy((u8 *)fwControl->buffer, > - (u8 *)pm8001_ha->fw_image->data + sizeRead, > - (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE); > - sizeRead += > - (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE; > - } else { > - memcpy((u8 *)fwControl->buffer, > - (u8 *)pm8001_ha->fw_image->data + sizeRead, > - IOCTL_BUF_SIZE); > - sizeRead += IOCTL_BUF_SIZE; > - } > - > - pm8001_ha->nvmd_completion = &completion; > - ret = PM8001_CHIP_DISP->fw_flash_update_req(pm8001_ha, payload); > - if (ret) { > - pm8001_ha->fw_status = FAIL_OUT_MEMORY; > - goto out; > - } > - wait_for_completion(&completion); > - if (fwControl->retcode > FLASH_UPDATE_IN_PROGRESS) { > - pm8001_ha->fw_status = fwControl->retcode; > - ret = -EFAULT; > - goto out; > - } > + /* > + * for the last chunk of data in case file size is > + * not even with 4k, load only the rest > + */ > + > + fc_len = (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE; I would move this line down, right above the "if" where the variable is used. > + read_buf = (u8 *)pm8001_ha->fw_image->data + sizeRead; > + > + if (((loopcount-loopNumber) == 1) && fc_len) { While at it, please add spaces around the "-" and remove the unnecessary parenthesis: if (loopcount - loopNumber == 1 && fc_len) { > + fwControl->len = fc_len; > + memcpy((u8 *)fwControl->buffer, read_buf, fc_len); > + sizeRead += fc_len; > + } else { > + memcpy((u8 *)fwControl->buffer, read_buf, IOCTL_BUF_SIZE); > + sizeRead += IOCTL_BUF_SIZE; > + } > + > + pm8001_ha->nvmd_completion = &completion; > + ret = PM8001_CHIP_DISP->fw_flash_update_req(pm8001_ha, payload); > + if (ret) { > + pm8001_ha->fw_status = FAIL_OUT_MEMORY; > + goto out; > + } > + wait_for_completion(&completion); > + if (fwControl->retcode > FLASH_UPDATE_IN_PROGRESS) { > + pm8001_ha->fw_status = fwControl->retcode; > + ret = -EFAULT; > + goto out; > + } > } > } > out: With that fixed, looks good. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c index 41a63c9b719b..0d9533ba8d27 100644 --- a/drivers/scsi/pm8001/pm8001_ctl.c +++ b/drivers/scsi/pm8001/pm8001_ctl.c @@ -727,6 +727,8 @@ static int pm8001_update_flash(struct pm8001_hba_info *pm8001_ha) u32 sizeRead = 0; u32 ret = 0; u32 length = 1024 * 16 + sizeof(*payload) - 1; + u32 fc_len = 0; + u8 *read_buf; if (pm8001_ha->fw_image->size < 28) { pm8001_ha->fw_status = FAIL_FILE_SIZE; @@ -755,36 +757,35 @@ static int pm8001_update_flash(struct pm8001_hba_info *pm8001_ha) fwControl->retcode = 0;/* OUT */ fwControl->offset = loopNumber * IOCTL_BUF_SIZE;/*OUT */ - /* for the last chunk of data in case file size is not even with - 4k, load only the rest*/ - if (((loopcount-loopNumber) == 1) && - ((partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE)) { - fwControl->len = - (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE; - memcpy((u8 *)fwControl->buffer, - (u8 *)pm8001_ha->fw_image->data + sizeRead, - (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE); - sizeRead += - (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE; - } else { - memcpy((u8 *)fwControl->buffer, - (u8 *)pm8001_ha->fw_image->data + sizeRead, - IOCTL_BUF_SIZE); - sizeRead += IOCTL_BUF_SIZE; - } - - pm8001_ha->nvmd_completion = &completion; - ret = PM8001_CHIP_DISP->fw_flash_update_req(pm8001_ha, payload); - if (ret) { - pm8001_ha->fw_status = FAIL_OUT_MEMORY; - goto out; - } - wait_for_completion(&completion); - if (fwControl->retcode > FLASH_UPDATE_IN_PROGRESS) { - pm8001_ha->fw_status = fwControl->retcode; - ret = -EFAULT; - goto out; - } + /* + * for the last chunk of data in case file size is + * not even with 4k, load only the rest + */ + + fc_len = (partitionSize + HEADER_LEN) % IOCTL_BUF_SIZE; + read_buf = (u8 *)pm8001_ha->fw_image->data + sizeRead; + + if (((loopcount-loopNumber) == 1) && fc_len) { + fwControl->len = fc_len; + memcpy((u8 *)fwControl->buffer, read_buf, fc_len); + sizeRead += fc_len; + } else { + memcpy((u8 *)fwControl->buffer, read_buf, IOCTL_BUF_SIZE); + sizeRead += IOCTL_BUF_SIZE; + } + + pm8001_ha->nvmd_completion = &completion; + ret = PM8001_CHIP_DISP->fw_flash_update_req(pm8001_ha, payload); + if (ret) { + pm8001_ha->fw_status = FAIL_OUT_MEMORY; + goto out; + } + wait_for_completion(&completion); + if (fwControl->retcode > FLASH_UPDATE_IN_PROGRESS) { + pm8001_ha->fw_status = fwControl->retcode; + ret = -EFAULT; + goto out; + } } } out:
Eliminate the follow smatch warning: drivers/scsi/pm8001/pm8001_ctl.c:760 pm8001_update_flash() warn: inconsistent indenting Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> --- Changes in v2: --According to Damien's suggestion 1) Start multi-line comments with a line that has only "/*". 2) Align the conditions together to make this code more readable. 3) Using a local variable for value which is calculated too many times. drivers/scsi/pm8001/pm8001_ctl.c | 61 ++++++++++++++++---------------- 1 file changed, 31 insertions(+), 30 deletions(-)