Re: [PATCH 1/2] btrfs-progs: check for no result before using results

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 17.12.18 г. 5:13 ч., Anand Jain wrote:
> User space understands the ioctl BTRFS_IOC_DEV_REPLACE command status
> using the struct btrfs_ioctl_dev_replace_args::result, and so userspace
> initializes this to BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT, so exclude
> this value in checking for the error.
> 
> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> ---
>  cmds-replace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/cmds-replace.c b/cmds-replace.c
> index b30e6c781e64..42de4de8c031 100644
> --- a/cmds-replace.c
> +++ b/cmds-replace.c
> @@ -296,6 +296,8 @@ static int cmd_replace_start(int argc, char **argv)
>  		}
>  
>  		if (start_args.result !=
> +		    BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT &&
> +		    start_args.result !=

While this change is OK, it really is redundant, since we do
IOC_DEV_REPLACE with CMD_STATUS, meaning in kernel space we always call
btrfs_dev_replace_status which always overwrites ->result member.


Also looking at the other 3 cmds available for this IOCTL it's always
guaranteed for ->result to be overwritten if it executes btrfs code.

OTOH  if the capable, memdup or an unrecognised ->cmd  is detected then
an ordinary error code is returned, in which case the ret < 0 check
executes and laves via "leave_with_error" label.

While your patch is OK code wise it's really a no op

>  		    BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_ERROR) {
>  			error("ioctl(DEV_REPLACE_START) on '%s' returns error: %s",
>  				path,
> 



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux