| Message ID | 1320061773-10634-1-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> writes: > Markus Armbruster <armbru@redhat.com> sent fixes for va_list vararg > issues in v9fs_string_alloc_printf(). It turns out the function > duplicates g_vasprintf() and can therefore be eliminated entirely. > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > --- > hw/9pfs/virtio-9p.c | 103 ++------------------------------------------------- > 1 files changed, 4 insertions(+), 99 deletions(-) *Much* better. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On Mon, 31 Oct 2011 11:49:33 +0000, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > Markus Armbruster <armbru@redhat.com> sent fixes for va_list vararg > issues in v9fs_string_alloc_printf(). It turns out the function > duplicates g_vasprintf() and can therefore be eliminated entirely. > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > --- > hw/9pfs/virtio-9p.c | 103 ++------------------------------------------------- > 1 files changed, 4 insertions(+), 99 deletions(-) > > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > index 8b6813f..253919b 100644 > --- a/hw/9pfs/virtio-9p.c > +++ b/hw/9pfs/virtio-9p.c > @@ -11,6 +11,9 @@ > * > */ > > +#include <glib.h> > +#include <glib/gprintf.h> > + > #include "hw/virtio.h" > #include "hw/pc.h" > #include "qemu_socket.h" > @@ -161,114 +164,16 @@ void v9fs_string_null(V9fsString *str) > v9fs_string_free(str); > } > > -static int number_to_string(void *arg, char type) > -{ > - unsigned int ret = 0; > - > - switch (type) { > - case 'u': { > - unsigned int num = *(unsigned int *)arg; > - > - do { > - ret++; > - num = num/10; > - } while (num); > - break; > - } > - case 'U': { > - unsigned long num = *(unsigned long *)arg; > - do { > - ret++; > - num = num/10; > - } while (num); > - break; > - } > - default: > - printf("Number_to_string: Unknown number format\n"); > - return -1; > - } > - > - return ret; > -} > - > -static int GCC_FMT_ATTR(2, 0) > -v9fs_string_alloc_printf(char **strp, const char *fmt, va_list ap) > -{ > - va_list ap2; > - char *iter = (char *)fmt; > - int len = 0; > - int nr_args = 0; > - char *arg_char_ptr; > - unsigned int arg_uint; > - unsigned long arg_ulong; > - > - /* Find the number of %'s that denotes an argument */ > - for (iter = strstr(iter, "%"); iter; iter = strstr(iter, "%")) { > - nr_args++; > - iter++; > - } > - > - len = strlen(fmt) - 2*nr_args; > - > - if (!nr_args) { > - goto alloc_print; > - } > - > - va_copy(ap2, ap); > - > - iter = (char *)fmt; > - > - /* Now parse the format string */ > - for (iter = strstr(iter, "%"); iter; iter = strstr(iter, "%")) { > - iter++; > - switch (*iter) { > - case 'u': > - arg_uint = va_arg(ap2, unsigned int); > - len += number_to_string((void *)&arg_uint, 'u'); > - break; > - case 'l': > - if (*++iter == 'u') { > - arg_ulong = va_arg(ap2, unsigned long); > - len += number_to_string((void *)&arg_ulong, 'U'); > - } else { > - return -1; > - } > - break; > - case 's': > - arg_char_ptr = va_arg(ap2, char *); > - len += strlen(arg_char_ptr); > - break; > - case 'c': > - len += 1; > - break; > - default: > - fprintf(stderr, > - "v9fs_string_alloc_printf:Incorrect format %c", *iter); > - return -1; > - } > - iter++; > - } > - > -alloc_print: > - *strp = g_malloc((len + 1) * sizeof(**strp)); > - > - return vsprintf(*strp, fmt, ap); > -} > - > void GCC_FMT_ATTR(2, 3) > v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) > { > va_list ap; > - int err; > > v9fs_string_free(str); > > va_start(ap, fmt); > - err = v9fs_string_alloc_printf(&str->data, fmt, ap); > - BUG_ON(err == -1); > + str->size = g_vasprintf(&str->data, fmt, ap); > va_end(ap); > - > - str->size = err; > } > > void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) > -- > 1.7.7 >
On Mon, Oct 31, 2011 at 11:28:45PM +0530, Aneesh Kumar K.V wrote: > On Mon, 31 Oct 2011 11:49:33 +0000, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > > Markus Armbruster <armbru@redhat.com> sent fixes for va_list vararg > > issues in v9fs_string_alloc_printf(). It turns out the function > > duplicates g_vasprintf() and can therefore be eliminated entirely. > > > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> Do you want to take this into your 9pfs tree? Stefan
On Tue, 1 Nov 2011 07:50:51 +0000, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > On Mon, Oct 31, 2011 at 11:28:45PM +0530, Aneesh Kumar K.V wrote: > > On Mon, 31 Oct 2011 11:49:33 +0000, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > > > Markus Armbruster <armbru@redhat.com> sent fixes for va_list vararg > > > issues in v9fs_string_alloc_printf(). It turns out the function > > > duplicates g_vasprintf() and can therefore be eliminated entirely. > > > > > > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > > > > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > > Do you want to take this into your 9pfs tree? Will push this through v9fs.git Thanks -aneesh
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index 8b6813f..253919b 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -11,6 +11,9 @@ * */ +#include <glib.h> +#include <glib/gprintf.h> + #include "hw/virtio.h" #include "hw/pc.h" #include "qemu_socket.h" @@ -161,114 +164,16 @@ void v9fs_string_null(V9fsString *str) v9fs_string_free(str); } -static int number_to_string(void *arg, char type) -{ - unsigned int ret = 0; - - switch (type) { - case 'u': { - unsigned int num = *(unsigned int *)arg; - - do { - ret++; - num = num/10; - } while (num); - break; - } - case 'U': { - unsigned long num = *(unsigned long *)arg; - do { - ret++; - num = num/10; - } while (num); - break; - } - default: - printf("Number_to_string: Unknown number format\n"); - return -1; - } - - return ret; -} - -static int GCC_FMT_ATTR(2, 0) -v9fs_string_alloc_printf(char **strp, const char *fmt, va_list ap) -{ - va_list ap2; - char *iter = (char *)fmt; - int len = 0; - int nr_args = 0; - char *arg_char_ptr; - unsigned int arg_uint; - unsigned long arg_ulong; - - /* Find the number of %'s that denotes an argument */ - for (iter = strstr(iter, "%"); iter; iter = strstr(iter, "%")) { - nr_args++; - iter++; - } - - len = strlen(fmt) - 2*nr_args; - - if (!nr_args) { - goto alloc_print; - } - - va_copy(ap2, ap); - - iter = (char *)fmt; - - /* Now parse the format string */ - for (iter = strstr(iter, "%"); iter; iter = strstr(iter, "%")) { - iter++; - switch (*iter) { - case 'u': - arg_uint = va_arg(ap2, unsigned int); - len += number_to_string((void *)&arg_uint, 'u'); - break; - case 'l': - if (*++iter == 'u') { - arg_ulong = va_arg(ap2, unsigned long); - len += number_to_string((void *)&arg_ulong, 'U'); - } else { - return -1; - } - break; - case 's': - arg_char_ptr = va_arg(ap2, char *); - len += strlen(arg_char_ptr); - break; - case 'c': - len += 1; - break; - default: - fprintf(stderr, - "v9fs_string_alloc_printf:Incorrect format %c", *iter); - return -1; - } - iter++; - } - -alloc_print: - *strp = g_malloc((len + 1) * sizeof(**strp)); - - return vsprintf(*strp, fmt, ap); -} - void GCC_FMT_ATTR(2, 3) v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) { va_list ap; - int err; v9fs_string_free(str); va_start(ap, fmt); - err = v9fs_string_alloc_printf(&str->data, fmt, ap); - BUG_ON(err == -1); + str->size = g_vasprintf(&str->data, fmt, ap); va_end(ap); - - str->size = err; } void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs)
Markus Armbruster <armbru@redhat.com> sent fixes for va_list vararg issues in v9fs_string_alloc_printf(). It turns out the function duplicates g_vasprintf() and can therefore be eliminated entirely. Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> --- hw/9pfs/virtio-9p.c | 103 ++------------------------------------------------- 1 files changed, 4 insertions(+), 99 deletions(-)