Re: [PATCH] pasta.c: modify hostname when detaching new namespace

[Stefano Brivio <sbrivio@redhat.com>]

Danish, it would be easier if you answered inline. If Gmail is making
it hard, perhaps switch to an email client (I use claws-mail)? Anyway,
it's not a big issue for the moment:

On Thu, 23 May 2024 19:22:16 +0530
Danish Prakash <contact@danishpraka.sh> wrote:

> Thanks for the review and the links.
> 
> > +       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,

Wait, I didn't suggest if (!gethostname(...)), I suggested
gethostname(...). The ! there is yours. :)

> doesn't work in cases where the hostname is long enough (~>58 chars).
> It works fine for shorter hostnames.

In any case, it depends on how you define "doesn't work". What should
we do if the original hostname is long enough that we can't prefix
"pasta-" while fitting in 63 characters?

Append it anyway and truncate the original hostname (what my line did),
or leave it like it is (what your snippet does, I guess)? It's a matter
of taste I'd say.

> 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).

...then the NOTES section disappeared and this ended up in "ERRORS", in
the Linux man-pages:

       ENAMETOOLONG
              (glibc gethostname()) len is smaller than the actual size.
              (Before glibc 2.1, glibc uses EINVAL for this case.)

> ...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]:

...no, I don't think you're missing out on any edge case, you simply
missed that part of the man page. Note that we need to play nicely with
other C libraries too (especially musl), so error or not, we should do
the right/same thing.

Perhaps most robust approach:

	if (!gethostname(hostname + sizeof(HOSTNAME_PREFIX) - 1,
			 HOST_NAME_MAX + 1 - sizeof(HOSTNAME_PREFIX)) ||
	    errno == ENAMETOOLONG) {

so that if it's glibc, and it truncates, we'll just go ahead with our
truncated name, but not if there's any other error.

Note that you don't strictly need the NULL byte at the end, see
sethostname(2) -- just make sure you don't print it or anything like
that.

-- 
Stefano