Re: [systemd-devel] [PATCH v3] hostname: Allow comments in /etc/hostname

2015-05-19 Thread Lennart Poettering
On Tue, 19.05.15 07:56, Martin Pitt (martin.p...@ubuntu.com) wrote:

> Hey Lennart,
> 
> Lennart Poettering [2015-05-18 17:12 +0200]:
> > This code will result in ENOENT being returned on OOM... That's not right...
> 
> Fixed.
> 
> > Also consider using strstrip() here.
> 
> hostname_cleanup() already filters out whitespace, so I don't think
> that's needed?

true!

> > Given that this same code is needed twice in different components,
> > please add a new call read_etc_hostname() to
> > src/shared/hostname-util.c (which I just added to git). It should then
> > also do hostname_cleanup() on the name, and thus pretty much replace
> > read_and_strip_hostname() entirely in hostname-setup.c.
> 
> Done. However, I called the function read_hostname_config() and give
> it a path argument. This is mostly so that we can write tests for it,
> but maybe one of these days we actually want to use it for other
> purposes (reading hostnames from container root fs).

Thanks! Applied!

Lennart

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


Re: [systemd-devel] [PATCH v3] hostname: Allow comments in /etc/hostname

2015-05-18 Thread systemd github import bot
Patchset imported to github.
Pull request:


--
Generated by https://github.com/haraldh/mail2git
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH v3] hostname: Allow comments in /etc/hostname

2015-05-18 Thread Martin Pitt
Hey Lennart,

Lennart Poettering [2015-05-18 17:12 +0200]:
> This code will result in ENOENT being returned on OOM... That's not right...

Fixed.

> Also consider using strstrip() here.

hostname_cleanup() already filters out whitespace, so I don't think
that's needed?

> Given that this same code is needed twice in different components,
> please add a new call read_etc_hostname() to
> src/shared/hostname-util.c (which I just added to git). It should then
> also do hostname_cleanup() on the name, and thus pretty much replace
> read_and_strip_hostname() entirely in hostname-setup.c.

Done. However, I called the function read_hostname_config() and give
it a path argument. This is mostly so that we can write tests for it,
but maybe one of these days we actually want to use it for other
purposes (reading hostnames from container root fs).

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From ef807f60409b26ab1168cef9df5f5de553e4259c Mon Sep 17 00:00:00 2001
From: Martin Pitt 
Date: Tue, 19 May 2015 07:49:56 +0200
Subject: [PATCH] hostname: Allow comments in /etc/hostname

The hostname(1) tool allows comments in /etc/hostname. Introduce a new
read_hostname_config() in hostname-util which reads a hostname configuration
file like /etc/hostname, strips out comments, whitespace, and cleans the
hostname. Use it in hostname-setup.c and hostnamed and remove duplicated code.

Update hostname manpage. Add tests.

https://launchpad.net/bugs/1053048
---
 man/hostname.xml   | 11 ++-
 src/core/hostname-setup.c  | 24 +--
 src/hostname/hostnamed.c   |  2 +-
 src/shared/hostname-util.c | 33 
 src/shared/hostname-util.h |  2 ++
 src/test/test-util.c   | 47 ++
 6 files changed, 90 insertions(+), 29 deletions(-)

diff --git a/man/hostname.xml b/man/hostname.xml
index 5d3d46d..9688450 100644
--- a/man/hostname.xml
+++ b/man/hostname.xml
@@ -57,11 +57,12 @@
 name of the local system that is set during boot using the
 sethostname2
 system call. It should contain a single newline-terminated
-hostname string. The hostname may be a free-form string up to 64
-characters in length; however, it is recommended that it consists
-only of 7-bit ASCII lower-case characters and no spaces or dots,
-and limits itself to the format allowed for DNS domain name
-labels, even though this is not a strict requirement.
+hostname string.  Comments (lines starting with a `#') are ignored.
+The hostname may be a free-form string up to 64 characters in length;
+however, it is recommended that it consists only of 7-bit ASCII lower-case
+characters and no spaces or dots, and limits itself to the format allowed
+for DNS domain name labels, even though this is not a strict
+requirement.
 
 Depending on the operating system, other configuration files
 might be checked for configuration of the hostname as well,
diff --git a/src/core/hostname-setup.c b/src/core/hostname-setup.c
index 217f201..932ddbf 100644
--- a/src/core/hostname-setup.c
+++ b/src/core/hostname-setup.c
@@ -30,35 +30,13 @@
 #include "hostname-util.h"
 #include "hostname-setup.h"
 
-static int read_and_strip_hostname(const char *path, char **hn) {
-char *s;
-int r;
-
-assert(path);
-assert(hn);
-
-r = read_one_line_file(path, &s);
-if (r < 0)
-return r;
-
-hostname_cleanup(s, false);
-
-if (isempty(s)) {
-free(s);
-return -ENOENT;
-}
-
-*hn = s;
-return 0;
-}
-
 int hostname_setup(void) {
 int r;
 _cleanup_free_ char *b = NULL;
 const char *hn;
 bool enoent = false;
 
-r = read_and_strip_hostname("/etc/hostname", &b);
+r = read_hostname_config("/etc/hostname", &b);
 if (r < 0) {
 if (r == -ENOENT)
 enoent = true;
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index ab9ddc7..7ff3a4e 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -96,7 +96,7 @@ static int context_read_data(Context *c) {
 if (!c->data[PROP_HOSTNAME])
 return -ENOMEM;
 
-r = read_one_line_file("/etc/hostname", &c->data[PROP_STATIC_HOSTNAME]);
+r = read_hostname_config("/etc/hostname", &c->data[PROP_STATIC_HOSTNAME]);
 if (r < 0 && r != -ENOENT)
 return r;
 
diff --git a/src/shared/hostname-util.c b/src/shared/hostname-util.c
index 2998fdf..e336f26 100644
--- a/src/shared/hostname-util.c
+++ b/src/shared/hostname-util.c
@@ -158,3 +158,36 @@ int sethostname_idempotent(const char *s) {
 
 return 1;
 }
+
+int read_hostname_config(const char *path, char **hostname) {
+_cleanup_fclose_ FILE *f = NULL;
+char l[LINE_MAX]