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

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to