| Message ID | 567f969602c1742e23c7760944e909346b2d012b.1592846572.git.mprivozn@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qga: Ditch g_get_host_name() | expand |
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
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); > +} >
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
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
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 --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); +}
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(+)