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

[Lennart Poettering]

On Tue, 10.03.15 17:34, Alban Crequy (alban.crequy at gmail.com) wrote:

>  
> -                r = renameat2(AT_FDCWD, i->temp_path, AT_FDCWD, i->final_path, RENAME_NOREPLACE);
> +                r = rename_noreplace(AT_FDCWD, i->temp_path, AT_FDCWD, i->final_path);
>                  if (r < 0) {
>                          r = log_error_errno(errno, "Failed to move RAW file into place: %m");
>                          goto finish;

If rename_noreplace() would return "-errno" like all the other calls
we define, then this would become:

if (r < 0) {
        log_error_errno(r, "Failed to move RAW file into place: %m");
        goto finish;
}

Which is both shorter and more inline with the rest of our code...

> -        if (renameat2(AT_FDCWD, t, AT_FDCWD, to, replace ? 0 : RENAME_NOREPLACE) < 0) {
> +        if (replace) {
> +                r = renameat(AT_FDCWD, t, AT_FDCWD, to);
> +        } else {
> +                r = rename_noreplace(AT_FDCWD, t, AT_FDCWD, to);
> +        }
> +        if (r < 0) {

Please, no {} for single-line if blocks. See CODING_STYLE.

> +        ret = unlinkat(olddirfd, oldpath, 0);
> +        if (ret < 0)
> +                unlinkat(newdirfd, newpath, 0);

Recently we started prefixing calls like this where we knowingly
ignore the return value with casts to (void). This tells code checkers
like Coverity that we *knowingly* ignore the error condition
here. Hence:

        (void) unlinkat(newdirfd, newpath, 0);

instead of:

        unlinkat(newdirfd, newpath, 0);

Lennart

-- 
Lennart Poettering, Red Hat