Re: [PATCH 03/13] xfs: delegate dqget input checks to helper function

[Christoph Hellwig <hch@xxxxxx>]

> +	if (!XFS_IS_QUOTA_RUNNING(mp)) {
> +		ASSERT(0);
> +		return -ESRCH;
> +	}

Maybe something like:

	if (WARN_ON_ONCE(!XFS_IS_QUOTA_RUNNING(mp)))
		return -ESRCH;

here and for the other ASSERT(0) case?

> +	if ((!XFS_IS_UQUOTA_ON(mp) && type == XFS_DQ_USER) ||
> +	    (!XFS_IS_PQUOTA_ON(mp) && type == XFS_DQ_PROJ) ||
> +	    (!XFS_IS_GQUOTA_ON(mp) && type == XFS_DQ_GROUP)) {
> +		return -ESRCH;
> +	}

No need for the curly braces.

> +
> +	if (type != XFS_DQ_USER &&
> +	    type != XFS_DQ_PROJ &&
> +	    type != XFS_DQ_GROUP) {
> +		ASSERT(0);
> +		return -EINVAL;

If we really care about these checks why not structure it as something
like:

	switch (type) {
	case XFS_DQ_USER:
		if (!XFS_IS_UQUOTA_ON(mp))
			return -ESRCH
		return 0;
	case XFS_DQ_PROJ:
		if (!XFS_IS_PQUOTA_ON(mp))
			return -ESRCH
		return 0;
	case XFS_DQ_GROUP:
		if (!XFS_IS_GQUOTA_ON(mp))
			return -ESRCH
		return 0;
	default:
		WARN_ON_ONCE(1);
		return -EINVAL;

Otherwise looks fine:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html