Re: [PATCH v4 15/16] block: sed-opal: don't repeat opal_discovery0 in each steps array

From: Christoph Hellwig
Date: Mon Feb 04 2019 - 10:01:38 EST


> + /* first do a discovery0 */
> + error = opal_discovery0_step(dev);
>
> + for (state = 0; !error && state < n_steps; state++)
> + error = execute_step(dev, &steps[state], state);
> +
> + /*
> + * For each OPAL command the first step in steps starts some sort of
> + * session. If an error occurred in the initial discovery0 or if an
> + * error occurred in the first step (and thus stopping the loop with
> + * state == 1) then there was an error before or during the attempt to
> + * start a session. Therefore we shouldn't attempt to terminate a
> + * session, as one has not yet been created.
> + */
> + if (error && state > 1)
> + end_opal_session_error(dev);
>
> return error;

The flow here is a little too condensed for my taste. Why not the
plain obvoious, if a little longer:

error = error = opal_discovery0_step(dev);
if (error)
return error;

for (state = 0; state < n_steps; state++) {
error = execute_step(dev, &steps[state], state);
if (error)
goto out_error;
}

return 0;

out_error:
if (state > 1)
end_opal_session_error(dev);
return error;

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>