+ if (!gethostname(hostname +
sizeof(HOSTNAME_PREFIX) - 1, HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFIX))) {
+ if (sethostname(hostname, strlen(hostname)))
+ debug("Unable to set pasta-prefixed hostname");
}
The above snippet, although it looks correct, doesn't work in cases
where the hostname is long enough (~>58 chars). It works fine for
shorter hostnames. The call to `gethostname()` sets errno to 36
(ENAMETOOLONG). Upon looking further, it seems that even though the
manpage for `gethostname(char *name, size_t len)` states..
If the null-terminated hostname is too large to fit,
then the name is truncated, and no error is returned (but see NOTES below).
...It would still throw ENAMETOOLONG if the value of len is smaller
than `strlen(hostname)+1` (referring to the snippet below). I'm not
sure if I'm missing out on an edge case while calculating the
boundaries here because the `memcpy` call is certainly truncating the
hostname[1] before ending up returning ENAMETOOLONG, which seems
conflicting[1]:
int
__gethostname (char *name, size_t len)
{
struct utsname buf;
size_t node_len;
if (__uname (&buf))
return -1;
node_len = strlen (buf.nodename) + 1;
memcpy (name, buf.nodename, len < node_len ? len : node_len);
if (node_len > len)
{
__set_errno (ENAMETOOLONG);
return -1;
}
return 0;
}
[1] -
On Mon, 20 May 2024 14:05:58 +0530
Danish Prakash <contact(a)danishpraka.sh> wrote:
When invoking pasta without any arguments,
it's difficult
to tell whether we are in the new namespace or not leaving
users a bit confused. This change modifies the host namespace
to add a prefix "pasta-" to make it a bit more obvious.
Thanks for the patch, it works for me:
sbrivio@maja:~/passt$ ./pasta --config-net
root@pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-pasta-pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-pasta-pasta-pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-pasta-pasta-pasta-pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-pasta-pasta-pasta-pasta-pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-maja:~/passt# ./pasta --config-net
root@pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-maja:~/passt# ./pasta
--config-net
root@pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-maja:~/passt# ./pasta
--config-net
root@pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-pasta-past:~/passt#
a few comments (all about brevity and style), below:
Signed-off-by: Danish Prakash
<contact(a)danishpraka.sh>
---
pasta.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/pasta.c b/pasta.c
index 31e1e00..840a2b1 100644
--- a/pasta.c
+++ b/pasta.c
@@ -180,6 +180,8 @@ static int pasta_spawn_cmd(void *arg)
{
const struct pasta_spawn_cmd_arg *a;
sigset_t set;
+ char hostname[HOST_NAME_MAX+1], pasta_hostname[HOST_NAME_MAX+1];
For coding style consistency: "HOST_NAME_MAX + 1" (with spaces).
+ char *hostname_prefix = "pasta-";
In passt, which follows the coding style of the Linux kernel for the
networking part, we order variables from the longest to the shortest.
Rationale:
https://hisham.hm/2018/06/16/when-listing-repeated-things-make-pyramids/
see also
https://lwn.net/Articles/758552/.
But actually, you don't need more than one variable here, see below.
/* We run in a detached PID and mount namespace: mount /proc over */
if (mount("", "/proc", "proc", 0, NULL))
@@ -188,6 +190,17 @@ static int pasta_spawn_cmd(void *arg)
if (write_file("/proc/sys/net/ipv4/ping_group_range", "0 0"))
warn("Cannot set ping_group_range, ICMP requests might fail");
+ if (gethostname(hostname, HOST_NAME_MAX+1) == 0) {
In general, unless one wants to stress that the return value could be a
number of positive or negative values, we just use for the common case
(0: success, -1: error):
if (!gethostname(...)) {
+ if ((strlen(hostname) +
strlen(hostname_prefix)) > HOST_NAME_MAX) {
+ hostname[strlen(hostname)-strlen(hostname_prefix)] =
'\0';
+ }
For consistency: no curly brackets for statements that fit a single
line.
+ sprintf(pasta_hostname,
"%s%s", hostname_prefix, hostname);
You could drop all this if you initialise the target variable directly, say:
char hostname[HOST_NAME_MAX + 1] = "pasta-";
then you can gethostname() on it + sizeof("pasta-") - 1 (NULL
terminating byte returned by sizeof()), directly.
Using a #define for "pasta-":
gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1,
HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFIX));
+
+ if (sethostname(pasta_hostname, strlen(pasta_hostname)) != 0) {
Same here about !sethostname() vs. sethostname() != 0, and curly
brackets.
+ warn("Unable to set
pasta-prefixed hostname");
+ }
+ }
+
/* Wait for the parent to be ready: see main() */
sigemptyset(&set);
sigaddset(&set, SIGUSR1);
--
Stefano