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

[Lennart Poettering]

On Tue, 21.10.14 18:32, Michal Sekletar (msekleta at 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