Re: [PATCH v4 2/2] RDMA/srp: Fix error return code in srp_parse_options()

[Bart Van Assche <bvanassche@acm.org>]

On 11/28/22 18:04, Wang Yufen wrote:
> In the previous while loop, "ret" may be assigned zero, , so the error

The word "iteration" is missing from the above sentence. Additionally, 
there is a double comma.

> return code may be incorrectly set to 0 instead of -EINVAL. Alse

Alse -> Also

>   		case SRP_OPT_QUEUE_SIZE:
> -			if (match_int(args, &token) || token < 1) {
> +			ret = match_int(args, &token);
> +			if (ret)
> +				goto out;
> +			if (token < 1) {
>   				pr_warn("bad queue_size parameter '%s'\n", p);
> +				ret = -EINVAL;
>   				goto out;
>   			}
>   			target->scsi_host->can_queue = token;

Why only to report "bad queue_size parameter" if ret == 0 && token < 1 
but not if ret < 0? This is a behavior change that has not been 
explained in the patch description.

> @@ -3490,25 +3496,34 @@ static int srp_parse_options(struct net *net, const char *buf,
>   			break;
>   
>   		case SRP_OPT_MAX_CMD_PER_LUN:
> -			if (match_int(args, &token) || token < 1) {
> +			ret = match_int(args, &token);
> +			if (ret)
> +				goto out;
> +			if (token < 1) {
>   				pr_warn("bad max cmd_per_lun parameter '%s'\n",
>   					p);
> +				ret = -EINVAL;
>   				goto out;
>   			}
>   			target->scsi_host->cmd_per_lun = token;
>   			break;

Same comment here and for many other changes below.

Thanks,

Bart.