[systemd-devel] [PATCH V3] cryptsetup: craft a unique ID with the source device

[Lennart Poettering]

On Fri, 29.05.15 18:24, harald at redhat.com (harald at redhat.com) wrote:

> +static int disk_major_minor(const char *path, char **ret) {
> +        struct stat st;
> +
> +        assert(path);
> +
> +        if (stat(path, &st) < 0)
> +                return -errno;
> +
> +        if (!S_ISBLK(st.st_mode))
> +                return -EINVAL;
> +
> +        if(asprintf(ret, "/dev/block/%d:%d", major(st.st_rdev), minor(st.st_rdev)) < 0) {
> +                return log_oom();
> +        }

There's a space missing between "if" and the opening "("...

Also, for single line if blocks we don't put {}, see CODING_STYLE.

Then, we should make sure that each function either logs about all its
errors, or about none and leaves logging to its callers. Currently you log
about OOM but not about the other errors. This means that the caller
will either print duplicate error messages or miss them
entirely... (also see CODING_STYLE)

In this case I think you should just "return -ENOMEM", and leave all
logging (or ignoring of any error) to the caller.

Otherwise looks fine.

Lennart

-- 
Lennart Poettering, Red Hat