On Tue, Feb 02, 2021 at 07:02:41PM +0100, Jan Kara wrote:
> Hum, let me think out loud. The path we pass to Q_QUOTAON is a path to
> quota file - unless the filesystem stores quota in hidden files in which
> case this argument is just ignored. You're right we could require that
> specifically for Q_QUOTAON, the mountpoint path would actually need to
> point to the quota file if it is relevant, otherwise anywhere in the
> appropriate filesystem. We don't allow quota file to reside on a different
> filesystem (for a past decade or so) so it should work fine.
>
> So the only problem I have is whether requiring the mountpoint argument to
> point quota file for Q_QUOTAON isn't going to be somewhat confusing to
> users. At the very least it would require some careful explanation in the
> manpage to explain the difference between quotactl_path() and quotactl()
> in this regard. But is saving the second path for Q_QUOTAON really worth the
> bother?
I find the doubled path argument a really horrible API, so I'd pretty
strongly prefer to avoid that.
> > Given how cheap quotactl_cmd_onoff and quotactl_cmd_write are we
> > could probably simplify this down do:
> >
> > if (quotactl_cmd_write(cmd)) {
>
> This needs to be (quotactl_cmd_write(cmd) || quotactl_cmd_onoff(cmd)).
> Otherwise I agree what you suggest is somewhat more readable given how
> small the function is.
The way I read quotactl_cmd_write, it only special cases a few commands
and returns 0 there, so we should not need the extra quotactl_cmd_onoff
call, as all those commands are not explicitly listed.
