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

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 24 May 2024 18:15:14 +0530
Danish Prakash <contact@danishpraka.sh> wrote:

> On 5/23/24 19:53, Stefano Brivio wrote:
> > 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:  
> 
> Yeah, something went wrong with that last one, sorry about that. I 
> switched to a new email and didn't properly set it up. Hopefully it'll 
> be better now.
> 
> > 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. :)  
> 
> I misread your point earlier i guess. My intent here is to change the 
> hostname (sethostname) if gethostname succeeds.

Sure, I understand, I just suggested a particular version of the
gethostname() call. I didn't comment on what's around it.

> >> 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.  
> 
> I'm not sure I follow this part here, in your "line", are you getting 
> and setting the hostname in two different conditionals?

Not really, I wasn't commenting on sethostname() at all, but in your
snippet you quoted above, yes, and I think it's fine, because there
might be other reasons why sethostname() fails.

> Because both 
> would be doing the same thing ie. truncating the hostname but given how 
> gethostname is implemented, the call would fail if len(hostname) > len 
> passed to gethostname...
> 
> > ...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.  
> 
> ...But this seems to do the job because gethostname returns the 
> truncated hostname *along* with ENAMETOOLONG in the edge case where 
> hostname is longer than the provided len, so as long as we're handling 
> and are okay with that error, we get the desired result.
> 
> I'll send along the updated patch shortly.
> 

-- 
Stefano