[systemd-devel] [PATCH] util: add rename_noreplace

[Lennart Poettering]

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
> success.

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
is.

Lennart

-- 
Lennart Poettering, Red Hat