Re: [PATCH v2 1/2] util: Introduce qemu_get_host_name()

[Michal Privoznik <mprivozn@redhat.com>]

On 6/22/20 7:38 PM, Daniel P. Berrangé wrote:
> On Mon, Jun 22, 2020 at 07:26:44PM +0200, Michal Privoznik wrote:
>> This function offers operating system agnostic way to fetch host
>> name. It is implemented for both POSIX-like and Windows systems.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   include/qemu/osdep.h | 10 ++++++++++
>>   util/oslib-posix.c   | 32 ++++++++++++++++++++++++++++++++
>>   util/oslib-win32.c   | 13 +++++++++++++
>>   3 files changed, 55 insertions(+)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index ff7c17b857..a795d46b28 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -607,4 +607,14 @@ static inline void qemu_reset_optind(void)
>>   #endif
>>   }
>>   
>> +/**
>> + * qemu_get_host_name:
>> + * @errp: Error object
>> + *
>> + * Operating system agnostic way of querying host name.
>> + *
>> + * Returns allocated hostname (caller should free), NULL on failure.
>> + */
>> +char *qemu_get_host_name(Error **errp);
>> +
>>   #endif
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index 916f1be224..865a3d71a7 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -761,3 +761,35 @@ void sigaction_invoke(struct sigaction *action,
>>       }
>>       action->sa_sigaction(info->ssi_signo, &si, NULL);
>>   }
>> +
>> +#ifndef HOST_NAME_MAX
>> +# ifdef _POSIX_HOST_NAME_MAX
>> +#  define HOST_NAME_MAX _POSIX_HOST_NAME_MAX
>> +# else
>> +#  define HOST_NAME_MAX 255
>> +# endif
>> +#endif
>> +
>> +char *qemu_get_host_name(Error **errp)
>> +{
>> +    long len = -1;
>> +    char *hostname;
>> +
>> +#ifdef _SC_HOST_NAME_MAX
>> +    len = sysconf(_SC_HOST_NAME_MAX);
>> +#endif /* _SC_HOST_NAME_MAX */
>> +
>> +    if (len < 0) {
>> +        len = HOST_NAME_MAX;
>> +    }
>> +
>> +    hostname = g_malloc0(len + 1);
> 
> Nitpick, generally qemu prefers g_new0
> 
>> +
>> +    if (gethostname(hostname, len) < 0) {
>> +        error_setg_errno(errp, errno,
>> +                         "cannot get hostname");
>> +        return NULL;
>> +    }
> 
> According to my man page, it is undefined by POSIX whether there's a
> trailing NUL when  hostname exceeds the buffer, so the paranoid thing
> todo is to add
> 
>    hostname[len] = '\0';

Isn't this guaranteed by allocating len + 1 bytes? I mean, g_malloc0() 
and g_new0() will memset() the memory to zero. And since I tell 
gethostname() the buf is only len bytes long I am guaranteed to have 0 
at the end of it, aren't I? Maybe I should put a comment just before 
g_malloc0() or g_new0() that documents this thought.

Michal