| Message ID | 20170823162004.27337-3-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi Marc-André, On 08/23/2017 01:19 PM, Marc-André Lureau wrote: > libvhost-user is meant to be free of glib dependency. Make sure it is > by droping qemu/osdep.h (which included glib.h) > > This fixes a bad malloc()/g_free() pair. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > contrib/libvhost-user/libvhost-user.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c > index 35fa0c5e56..bb294c6ef7 100644 > --- a/contrib/libvhost-user/libvhost-user.c > +++ b/contrib/libvhost-user/libvhost-user.c > @@ -13,11 +13,22 @@ > * later. See the COPYING file in the top-level directory. > */ > > -#include <qemu/osdep.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <unistd.h> > +#include <stdarg.h> > +#include <errno.h> > +#include <string.h> > +#include <assert.h> > +#include <inttypes.h> > +#include <sys/types.h> > +#include <sys/socket.h> > #include <sys/eventfd.h> > +#include <sys/mman.h> > #include <linux/vhost.h> > > #include "qemu/atomic.h" > +#include "qemu/compiler.h" > > #include "libvhost-user.h" > > @@ -34,6 +45,14 @@ > } \ > } while (0) > > +#ifndef MIN > +#define MIN(x, y) ({ \ > + typeof(x) _min1 = (x); \ > + typeof(y) _min2 = (y); \ > + (void) (&_min1 == &_min2); \ > + _min1 < _min2 ? _min1 : _min2; }) why not add this in qemu/compiler.h instead? > +#endif > + > static const char * > vu_request_to_string(int req) > { > @@ -81,7 +100,7 @@ vu_panic(VuDev *dev, const char *msg, ...) > va_list ap; > > va_start(ap, msg); > - buf = g_strdup_vprintf(msg, ap); > + vasprintf(&buf, msg, ap); > va_end(ap); > > dev->broken = true; > @@ -840,7 +859,7 @@ vu_dispatch(VuDev *dev) > success = true; > > end: > - g_free(vmsg.data); > + free(vmsg.data); > return success; > } > >
Hi ----- Original Message ----- > Hi Marc-André, > > On 08/23/2017 01:19 PM, Marc-André Lureau wrote: > > libvhost-user is meant to be free of glib dependency. Make sure it is > > by droping qemu/osdep.h (which included glib.h) > > > > This fixes a bad malloc()/g_free() pair. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > contrib/libvhost-user/libvhost-user.c | 25 ++++++++++++++++++++++--- > > 1 file changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/contrib/libvhost-user/libvhost-user.c > > b/contrib/libvhost-user/libvhost-user.c > > index 35fa0c5e56..bb294c6ef7 100644 > > --- a/contrib/libvhost-user/libvhost-user.c > > +++ b/contrib/libvhost-user/libvhost-user.c > > @@ -13,11 +13,22 @@ > > * later. See the COPYING file in the top-level directory. > > */ > > > > -#include <qemu/osdep.h> > > +#include <stdlib.h> > > +#include <stdio.h> > > +#include <unistd.h> > > +#include <stdarg.h> > > +#include <errno.h> > > +#include <string.h> > > +#include <assert.h> > > +#include <inttypes.h> > > +#include <sys/types.h> > > +#include <sys/socket.h> > > #include <sys/eventfd.h> > > +#include <sys/mman.h> > > #include <linux/vhost.h> > > > > #include "qemu/atomic.h" > > +#include "qemu/compiler.h" > > > > #include "libvhost-user.h" > > > > @@ -34,6 +45,14 @@ > > } \ > > } while (0) > > > > +#ifndef MIN > > +#define MIN(x, y) ({ \ > > + typeof(x) _min1 = (x); \ > > + typeof(y) _min2 = (y); \ > > + (void) (&_min1 == &_min2); \ > > + _min1 < _min2 ? _min1 : _min2; }) > > why not add this in qemu/compiler.h instead? It's a special case here, because libvhost-user doesn't depend on glib. The rest of qemu does. Since MIN/MAX is defined by glib, I don't think qemu/compiler.h should define it. > > > +#endif > > + > > static const char * > > vu_request_to_string(int req) > > { > > @@ -81,7 +100,7 @@ vu_panic(VuDev *dev, const char *msg, ...) > > va_list ap; > > > > va_start(ap, msg); > > - buf = g_strdup_vprintf(msg, ap); > > + vasprintf(&buf, msg, ap); > > va_end(ap); > > > > dev->broken = true; > > @@ -840,7 +859,7 @@ vu_dispatch(VuDev *dev) > > success = true; > > > > end: > > - g_free(vmsg.data); > > + free(vmsg.data); > > return success; > > } > > > > >
On 09/12/2017 10:13 AM, Marc-André Lureau wrote: > Hi > > ----- Original Message ----- >> Hi Marc-André, >> >> On 08/23/2017 01:19 PM, Marc-André Lureau wrote: >>> libvhost-user is meant to be free of glib dependency. Make sure it is >>> by droping qemu/osdep.h (which included glib.h) >>> >>> This fixes a bad malloc()/g_free() pair. >>> >>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>> --- >>> contrib/libvhost-user/libvhost-user.c | 25 ++++++++++++++++++++++--- >>> 1 file changed, 22 insertions(+), 3 deletions(-) >>> >>> diff --git a/contrib/libvhost-user/libvhost-user.c >>> b/contrib/libvhost-user/libvhost-user.c >>> index 35fa0c5e56..bb294c6ef7 100644 >>> --- a/contrib/libvhost-user/libvhost-user.c >>> +++ b/contrib/libvhost-user/libvhost-user.c >>> @@ -13,11 +13,22 @@ >>> * later. See the COPYING file in the top-level directory. >>> */ >>> /* this code avoids GLib dependency */ >>> -#include <qemu/osdep.h> >>> +#include <stdlib.h> >>> +#include <stdio.h> >>> +#include <unistd.h> >>> +#include <stdarg.h> >>> +#include <errno.h> >>> +#include <string.h> >>> +#include <assert.h> >>> +#include <inttypes.h> >>> +#include <sys/types.h> >>> +#include <sys/socket.h> >>> #include <sys/eventfd.h> >>> +#include <sys/mman.h> >>> #include <linux/vhost.h> >>> >>> #include "qemu/atomic.h" >>> +#include "qemu/compiler.h" >>> >>> #include "libvhost-user.h" >>> >>> @@ -34,6 +45,14 @@ >>> } \ >>> } while (0) >>> >>> +#ifndef MIN /* usually provided by GLib */ >>> +#define MIN(x, y) ({ \ >>> + typeof(x) _min1 = (x); \ >>> + typeof(y) _min2 = (y); \ >>> + (void) (&_min1 == &_min2); \ >>> + _min1 < _min2 ? _min1 : _min2; }) >> >> why not add this in qemu/compiler.h instead? > > It's a special case here, because libvhost-user doesn't depend on glib. The rest of qemu does. Since MIN/MAX is defined by glib, I don't think qemu/compiler.h should define it. OK, with some comments about GLib Independence: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >> >>> +#endif >>> + >>> static const char * >>> vu_request_to_string(int req) >>> { >>> @@ -81,7 +100,7 @@ vu_panic(VuDev *dev, const char *msg, ...) >>> va_list ap; >>> >>> va_start(ap, msg); >>> - buf = g_strdup_vprintf(msg, ap); >>> + vasprintf(&buf, msg, ap); >>> va_end(ap); >>> >>> dev->broken = true; >>> @@ -840,7 +859,7 @@ vu_dispatch(VuDev *dev) >>> success = true; >>> >>> end: >>> - g_free(vmsg.data); >>> + free(vmsg.data); >>> return success; >>> } >>> >>> >>
diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c index 35fa0c5e56..bb294c6ef7 100644 --- a/contrib/libvhost-user/libvhost-user.c +++ b/contrib/libvhost-user/libvhost-user.c @@ -13,11 +13,22 @@ * later. See the COPYING file in the top-level directory. */ -#include <qemu/osdep.h> +#include <stdlib.h> +#include <stdio.h> +#include <unistd.h> +#include <stdarg.h> +#include <errno.h> +#include <string.h> +#include <assert.h> +#include <inttypes.h> +#include <sys/types.h> +#include <sys/socket.h> #include <sys/eventfd.h> +#include <sys/mman.h> #include <linux/vhost.h> #include "qemu/atomic.h" +#include "qemu/compiler.h" #include "libvhost-user.h" @@ -34,6 +45,14 @@ } \ } while (0) +#ifndef MIN +#define MIN(x, y) ({ \ + typeof(x) _min1 = (x); \ + typeof(y) _min2 = (y); \ + (void) (&_min1 == &_min2); \ + _min1 < _min2 ? _min1 : _min2; }) +#endif + static const char * vu_request_to_string(int req) { @@ -81,7 +100,7 @@ vu_panic(VuDev *dev, const char *msg, ...) va_list ap; va_start(ap, msg); - buf = g_strdup_vprintf(msg, ap); + vasprintf(&buf, msg, ap); va_end(ap); dev->broken = true; @@ -840,7 +859,7 @@ vu_dispatch(VuDev *dev) success = true; end: - g_free(vmsg.data); + free(vmsg.data); return success; }
libvhost-user is meant to be free of glib dependency. Make sure it is by droping qemu/osdep.h (which included glib.h) This fixes a bad malloc()/g_free() pair. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- contrib/libvhost-user/libvhost-user.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-)