[systemd-devel] [PATCH 1/5] Factorize some machine-id-setup functions to be reused

[Lennart Poettering]

On Tue, 02.12.14 11:43, Didier Roche (didrocks at 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 (didrocks at 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 <didrocks at 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