diff mbox series

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

Message ID 567f969602c1742e23c7760944e909346b2d012b.1592846572.git.mprivozn@redhat.com (mailing list archive)
State New, archived
Headers show
Series qga: Ditch g_get_host_name() | expand

Commit Message

Michal Privoznik June 22, 2020, 5:26 p.m. UTC
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(+)

Comments

Daniel P. Berrangé June 22, 2020, 5:38 p.m. UTC | #1
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';

Regards,
Daniel
Philippe Mathieu-Daudé June 22, 2020, 5:46 p.m. UTC | #2
On 6/22/20 7:26 PM, 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.

free -> g_free?

> + */
> +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);
> +
> +    if (gethostname(hostname, len) < 0) {
> +        error_setg_errno(errp, errno,
> +                         "cannot get hostname");

Missing:

           g_free(hostname);

> +        return NULL;
> +    }
> +
> +    return hostname;
> +}
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index e9b14ab178..3b49d27297 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -808,3 +808,16 @@ bool qemu_write_pidfile(const char *filename, Error **errp)
>      }
>      return true;
>  }
> +
> +char *qemu_get_host_name(Error **errp)
> +{
> +    wchar_t tmp[MAX_COMPUTERNAME_LENGTH + 1];
> +    DWORD size = G_N_ELEMENTS(tmp);
> +
> +    if (GetComputerNameW(tmp, &size) == 0) {
> +        error_setg_win32(errp, GetLastError(), "failed close handle");
> +        return NULL;
> +    }
> +
> +    return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
> +}
>
Daniel P. Berrangé June 22, 2020, 5:49 p.m. UTC | #3
On Mon, Jun 22, 2020 at 07:46:08PM +0200, Philippe Mathieu-Daudé wrote:
> On 6/22/20 7:26 PM, 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.
> 
> free -> g_free?

free & g_free are guaranteed interchangable, given our min glib2 version.

Regards,
Daniel
Michal Privoznik June 22, 2020, 5:53 p.m. UTC | #4
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
Daniel P. Berrangé June 22, 2020, 5:54 p.m. UTC | #5
On Mon, Jun 22, 2020 at 07:53:40PM +0200, Michal Privoznik wrote:
> 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.

Oh right, yes, I'm mis-reading the code.

Regards,
Daniel
diff mbox series

Patch

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);
+
+    if (gethostname(hostname, len) < 0) {
+        error_setg_errno(errp, errno,
+                         "cannot get hostname");
+        return NULL;
+    }
+
+    return hostname;
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e9b14ab178..3b49d27297 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -808,3 +808,16 @@  bool qemu_write_pidfile(const char *filename, Error **errp)
     }
     return true;
 }
+
+char *qemu_get_host_name(Error **errp)
+{
+    wchar_t tmp[MAX_COMPUTERNAME_LENGTH + 1];
+    DWORD size = G_N_ELEMENTS(tmp);
+
+    if (GetComputerNameW(tmp, &size) == 0) {
+        error_setg_win32(errp, GetLastError(), "failed close handle");
+        return NULL;
+    }
+
+    return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
+}