Thu, Feb 29, 2024 at 04:10:27PM +0100, Nuno Sa kirjoitti:
> Use the new cleanup magic for handling mutexes in IIO. This allows us to
> greatly simplify some code paths.
...
> ret = iio_scan_mask_query(indio_dev, buffer, this_attr->address);
> if (ret < 0)
> - goto error_ret;
> - if (!state && ret) {
> - ret = iio_scan_mask_clear(buffer, this_attr->address);
> - if (ret)
> - goto error_ret;
> - } else if (state && !ret) {
> + return ret;
> +
> + if (state && ret)
> + return len;
I would leave the original checks. It's natural pattern
if (foo && !bar)
if (!foo && bar) // or 'else if' depending on the context
> + if (state)
> ret = iio_scan_mask_set(indio_dev, buffer, this_attr->address);
> - if (ret)
> - goto error_ret;
> - }
> + else
> + ret = iio_scan_mask_clear(buffer, this_attr->address);
> + if (ret)
> + return ret;
>
> -error_ret:
> - mutex_unlock(&iio_dev_opaque->mlock);
> -
> - return ret < 0 ? ret : len;
> + return len;
...
> /* Already in desired state */
> if (inlist == requested_state)
> - goto done;
> + return len;
Returning error code immediately is fine, but splitting return success seems to
me a bit too much. It is harder to follow (you really need to understand how many
"success" variants can be).
> if (requested_state)
> ret = __iio_update_buffers(indio_dev, buffer, NULL);
> else
> ret = __iio_update_buffers(indio_dev, NULL, buffer);
> + if (ret)
> + return ret;
>
> -done:
> - mutex_unlock(&iio_dev_opaque->mlock);
> - return (ret < 0) ? ret : len;
> + return len;
> }
--
With Best Regards,
Andy Shevchenko
