Re: [systemd-devel] [PATCH] util: introduce sethostname_idempotent

[Michal Sekletar]

On Tue, Oct 21, 2014 at 07:29:31PM +0200, Lennart Poettering wrote:
 On Tue, 21.10.14 18:32, Michal Sekletar (msekl...@redhat.com) wrote:
snip
 Go ahead and commit. Ideally with those two nitpicks fixed, but even
 if you don't it's OK.

sethostname_idempotent now returns 1 when hostname was changed. Also, I've
pushed follow-up fix-up for copy paste error, so we actually call sethostname()
on s not on buf which contains old hostname.

Michal

 
 Lennart
 
 -- 
 Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] util: introduce sethostname_idempotent

[Michal Sekletar]

Function queries system hostname and applies changes only when necessary. Also,
migrate all client of sethostname to sethostname_idempotent while at it.
---
 src/core/hostname-setup.c |  2 +-
 src/hostname/hostnamed.c  |  2 +-
 src/nspawn/nspawn.c   |  2 +-
 src/shared/util.c | 20 
 src/shared/util.h |  2 ++
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/core/hostname-setup.c b/src/core/hostname-setup.c
index 8aa1cff..57baa79 100644
--- a/src/core/hostname-setup.c
+++ b/src/core/hostname-setup.c
@@ -82,7 +82,7 @@ int hostname_setup(void) {
 hn = localhost;
 }
 
-if (sethostname(hn, strlen(hn))  0) {
+if (sethostname_idempotent(hn)  0) {
 log_warning(Failed to set hostname to %s: %m, hn);
 return -errno;
 }
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index b6b5d52..3e5d434 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -287,7 +287,7 @@ static int context_update_kernel_hostname(Context *c) {
 else
 hn = localhost;
 
-if (sethostname(hn, strlen(hn))  0)
+if (sethostname_idempotent(hn)  0)
 return -errno;
 
 return 0;
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index c567c8d..b6d9bc6 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -1289,7 +1289,7 @@ static int setup_hostname(void) {
 if (arg_share_system)
 return 0;
 
-if (sethostname(arg_machine, strlen(arg_machine))  0)
+if (sethostname_idempotent(arg_machine)  0)
 return -errno;
 
 return 0;
diff --git a/src/shared/util.c b/src/shared/util.c
index 5f6249e..cb3bd2f 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7164,3 +7164,23 @@ int free_and_strdup(char **p, const char *s) {
 
 return 0;
 }
+
+int sethostname_idempotent(const char *s) {
+int r = 0;
+char buf[HOST_NAME_MAX + 1] = {};
+
+assert(s);
+
+r = gethostname(buf, sizeof(buf));
+if (r  0)
+return -errno;
+
+if (streq(buf, s))
+return 0;
+
+r = sethostname(buf, strlen(buf));
+if (r  0)
+return -errno;
+
+return 0;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 21a90a4..10cdc7b 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -996,3 +996,5 @@ int unquote_first_word(const char **p, char **ret);
 int unquote_many_words(const char **p, ...) _sentinel_;
 
 int free_and_strdup(char **p, const char *s);
+
+int sethostname_idempotent(const char *s);
-- 
1.8.3.1

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

Re: [systemd-devel] [PATCH] util: introduce sethostname_idempotent

[Lennart Poettering]

On Tue, 21.10.14 18:32, Michal Sekletar (msekl...@redhat.com) wrote:

 Function queries system hostname and applies changes only when necessary. 
 Also,
 migrate all client of sethostname to sethostname_idempotent while at
 it.

Looks pretty good!

 +int sethostname_idempotent(const char *s) {
 +int r = 0;

Initializing this to 0 is not really necessary as you override it anyway.

 +char buf[HOST_NAME_MAX + 1] = {};
 +
 +assert(s);
 +
 +r = gethostname(buf, sizeof(buf));
 +if (r  0)
 +return -errno;
 +
 +if (streq(buf, s))
 +return 0;
 +
 +r = sethostname(buf, strlen(buf));
 +if (r  0)
 +return -errno;
 +
 +return 0;

Not that it would really matter here, but given that we return an int
here anyway we might as well return 1 if we made any change, so that a
future caller might use the return code as an indication whether the
host name was changed or not returning negative would then
indicate error, returning 0 would mean no change and returning
postiive would mean a change was made.

 +}
 diff --git a/src/shared/util.h b/src/shared/util.h
 index 21a90a4..10cdc7b 100644
 --- a/src/shared/util.h
 +++ b/src/shared/util.h
 @@ -996,3 +996,5 @@ int unquote_first_word(const char **p, char **ret);
  int unquote_many_words(const char **p, ...) _sentinel_;
  
  int free_and_strdup(char **p, const char *s);
 +
 +int sethostname_idempotent(const char *s);

Go ahead and commit. Ideally with those two nitpicks fixed, but even
if you don't it's OK.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel