[systemd-devel] [PATCH] util: add rename_noreplace
lennart at poettering.net
Tue Mar 10 09:34:21 PDT 2015
On Tue, 10.03.15 17:22, Alban Crequy (alban at endocode.com) wrote:
> >> + struct stat buf;
> >> + int ret;
> >> +
> >> + ret = renameat2(olddirfd, oldpath, newdirfd, newpath, RENAME_NOREPLACE);
> >> +
> >> + /* Even though renameat2() exists since Linux 3.15, btrfs added
> >> + * support for it later. If it is not implemented, fallback to another
> >> + * method. */
> >> + if (ret != -1 || errno != EINVAL)
> >> + return ret;
> > Hmm, not that it would matter much, but in systemd we so far mostly
> > check for < 0 rather then == -1 when looking for errors in syscalls...
> Ok. I'm changing for ret == 0 here though since we are checking for
I think checking for < 0 and >= 0 is a good idea, instead of
explicitly checking for == values...
> > Also, I think it would be a good idea to return "-errno" here, even
> > though the underlying syscalls don't do that. However, the "return
> > -errno" thing is pretty ubiquitously used in systemd, and at least as
> > useful as return the kernel's original -1 here...
> Callers print errno with log_error_errno(%m) so it seems easier to
> keep the same return values as renameat2.
The first argument of log_error_errno takes the error code that %m
resolves to. Hence, if you return -errno, then just pass that return
value to log_error_errno() as first argument, and all is good.
> Or do you want to give the caller the choice between using the return
> value or errno?
Well, I don't mind if errno is also set. But for our own calls we
generally return error as -errno. See CODING_STYLE. Functions should
not deviate needlessly from this logic, since it would be hard to know
what scheme is used where...
> > I figure we should explicitly return with an error on stuff that is
> > neither S_ISDIR, nor S_ISREG, since neither renameat() nor link()
> > would work on it, right?
> I was expecting linkat() to catch the errors. I tried to create hard
> links on socket, fifo and device files and it works fine. I don't mind
> either way.
Oh, that works? I didn't know. If it does, then leave the code like it
Lennart Poettering, Red Hat
More information about the systemd-devel