On Tue, 02.12.14 11:43, Didier Roche (didro...@ubuntu.com) wrote:
Looks good! Applied!
> Le 01/12/2014 18:38, Lennart Poettering a écrit :
> >On Mon, 24.11.14 12:35, Didier Roche (didro...@ubuntu.com) wrote:
> >
> >>+static int get_valid_machine_id(int fd, char id[34]) {
> >>+ assert(fd >= 0);
> >>+ assert(id);
> >>+
> >>+ if (loop_read(fd, id, 33, false) == 33 && id[32] == '\n') {
> >>+ id[32] = 0;
> >>+
> >>+ if (id128_is_valid(id)) {
> >>+ id[32] = '\n';
> >>+ id[33] = 0;
> >>+ return 0;
> >>+ }
> >>+ }
> >>+
> >>+ return -EINVAL;
> >>+}
> >As a matter of coding style we try hard to avoid functions that clobber
> >their parameters if they fail. Please follow the same scheme here.
> That makes sense, I rewrote the function to avoid this.
>
> Didier
> >From 7cf7322ab08c9434ba303d9958517f262b1797e0 Mon Sep 17 00:00:00 2001
> From: Didier Roche <didro...@ubuntu.com>
> Date: Mon, 24 Nov 2014 09:40:57 +0100
> Subject: [PATCH 1/5] Factorize some machine-id-setup functions to be reused
>
> ---
> src/core/machine-id-setup.c | 44 ++++++++++++++++++++++++++++++++++----------
> 1 file changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
> index 6710038..f3763ed 100644
> --- a/src/core/machine-id-setup.c
> +++ b/src/core/machine-id-setup.c
> @@ -157,6 +157,37 @@ static int generate(char id[34], const char *root) {
> return 0;
> }
>
> +static int get_valid_machine_id(int fd, char id[34]) {
> + char id_to_validate[34];
> +
> + assert(fd >= 0);
> + assert(id);
> +
> + if (loop_read(fd, id_to_validate, 33, false) == 33 &&
> id_to_validate[32] == '\n') {
> + id_to_validate[32] = 0;
> +
> + if (id128_is_valid(id_to_validate)) {
> + strncpy(id, id_to_validate, sizeof(id_to_validate));
> + id[32] = '\n';
> + id[33] = 0;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int write_machine_id(int fd, char id[34]) {
> + assert(fd >= 0);
> + assert(id);
> + lseek(fd, 0, SEEK_SET);
> +
> + if (loop_write(fd, id, 33, false) == 33)
> + return 0;
> +
> + return -errno;
> +}
> +
> int machine_id_setup(const char *root) {
> const char *etc_machine_id, *run_machine_id;
> _cleanup_close_ int fd = -1;
> @@ -207,13 +238,8 @@ int machine_id_setup(const char *root) {
> if (fstat(fd, &st) < 0)
> return log_error_errno(errno, "fstat() failed: %m");
>
> - if (S_ISREG(st.st_mode))
> - if (loop_read(fd, id, 33, false) == 33 && id[32] == '\n') {
> - id[32] = 0;
> -
> - if (id128_is_valid(id))
> - return 0;
> - }
> + if (S_ISREG(st.st_mode) && get_valid_machine_id(fd, id) == 0)
> + return 0;
>
> /* Hmm, so, the id currently stored is not useful, then let's
> * generate one */
> @@ -223,9 +249,7 @@ int machine_id_setup(const char *root) {
> return r;
>
> if (S_ISREG(st.st_mode) && writable) {
> - lseek(fd, 0, SEEK_SET);
> -
> - if (loop_write(fd, id, 33, false) == 33)
> + if (write_machine_id(fd, id) == 0)
> return 0;
> }
>
> --
> 2.1.3
>
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
