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
