Re: [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
On 8 September 2016 at 15:37, Eric Blakewrote: > On 08/05/2016 05:43 AM, Peter Maydell wrote: >> Some tests use the qtest protocol "memset" command with a zero >> size, expecting it to do nothing. However in the current code this >> will result in calling memset() with a NULL pointer, which is >> undefined behaviour. Detect and specially handle zero sizes to >> avoid this. >> >> Signed-off-by: Peter Maydell >> --- >> Looking at the code for the other commands that take a size >> ('read', 'write', 'b64read' and 'b64write' they all assume a >> non-zero size. I've left those alone though, somebody else can >> make them do nothing on zero size if they feel it's important.) > > I obviously missed reviewing this in time for 2.7, but looks reasonable > to me. > > Reviewed-by: Eric Blake Applied to master, thanks. -- PMM
Re: [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
On 08/05/2016 05:43 AM, Peter Maydell wrote: > Some tests use the qtest protocol "memset" command with a zero > size, expecting it to do nothing. However in the current code this > will result in calling memset() with a NULL pointer, which is > undefined behaviour. Detect and specially handle zero sizes to > avoid this. > > Signed-off-by: Peter Maydell> --- > Looking at the code for the other commands that take a size > ('read', 'write', 'b64read' and 'b64write' they all assume a > non-zero size. I've left those alone though, somebody else can > make them do nothing on zero size if they feel it's important.) I obviously missed reviewing this in time for 2.7, but looks reasonable to me. Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
Ping? (Now that 2.7 is out it would be nice to get rid of the clang warnings cluttering up my build logs :-)) thanks -- PMM On 5 August 2016 at 11:43, Peter Maydellwrote: > Some tests use the qtest protocol "memset" command with a zero > size, expecting it to do nothing. However in the current code this > will result in calling memset() with a NULL pointer, which is > undefined behaviour. Detect and specially handle zero sizes to > avoid this. > > Signed-off-by: Peter Maydell > --- > Looking at the code for the other commands that take a size > ('read', 'write', 'b64read' and 'b64write' they all assume a > non-zero size. I've left those alone though, somebody else can > make them do nothing on zero size if they feel it's important.) > > qtest.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/qtest.c b/qtest.c > index da4826c..ce4c6db 100644 > --- a/qtest.c > +++ b/qtest.c > @@ -133,6 +133,7 @@ static bool qtest_opened; > * < OK > * > * ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0. > + * For 'memset' a zero size is permitted and does nothing. > * > * DATA is an arbitrarily long hex number prefixed with '0x'. If it's > smaller > * than the expected size, the value will be zero filled at the end of the > data > @@ -493,10 +494,12 @@ static void qtest_process_command(CharDriverState *chr, > gchar **words) > len = strtoull(words[2], NULL, 0); > pattern = strtoull(words[3], NULL, 0); > > -data = g_malloc(len); > -memset(data, pattern, len); > -cpu_physical_memory_write(addr, data, len); > -g_free(data); > +if (len) { > +data = g_malloc(len); > +memset(data, pattern, len); > +cpu_physical_memory_write(addr, data, len); > +g_free(data); > +} > > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > -- > 2.7.4
[Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
Some tests use the qtest protocol "memset" command with a zero size, expecting it to do nothing. However in the current code this will result in calling memset() with a NULL pointer, which is undefined behaviour. Detect and specially handle zero sizes to avoid this. Signed-off-by: Peter Maydell--- Looking at the code for the other commands that take a size ('read', 'write', 'b64read' and 'b64write' they all assume a non-zero size. I've left those alone though, somebody else can make them do nothing on zero size if they feel it's important.) qtest.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/qtest.c b/qtest.c index da4826c..ce4c6db 100644 --- a/qtest.c +++ b/qtest.c @@ -133,6 +133,7 @@ static bool qtest_opened; * < OK * * ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0. + * For 'memset' a zero size is permitted and does nothing. * * DATA is an arbitrarily long hex number prefixed with '0x'. If it's smaller * than the expected size, the value will be zero filled at the end of the data @@ -493,10 +494,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words) len = strtoull(words[2], NULL, 0); pattern = strtoull(words[3], NULL, 0); -data = g_malloc(len); -memset(data, pattern, len); -cpu_physical_memory_write(addr, data, len); -g_free(data); +if (len) { +data = g_malloc(len); +memset(data, pattern, len); +cpu_physical_memory_write(addr, data, len); +g_free(data); +} qtest_send_prefix(chr); qtest_send(chr, "OK\n"); -- 2.7.4