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

[Danish Prakash <contact@danishpraka.sh>]

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.

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

-- 
danishpraka.sh