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

2015-05-29 Thread systemd github import bot
Patchset imported to github.
Pull request:
https://github.com/systemd-devs/systemd/compare/master...systemd-mailing-devs:1432916659-16919-1-git-send-email-harald%40redhat.com

--
Generated by https://github.com/haraldh/mail2git
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2015-05-29 Thread Lennart Poettering
On Fri, 29.05.15 18:24, har...@redhat.com (har...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel