Re: [PATCH V2 1/2] plugins: Fix resource leak in connect_socket()
On 2020/11/6 21:17, Eric Blake wrote:
> On 11/5/20 7:59 PM, AlexChen wrote:
>> Close the fd when the connect() fails.
>>
>> Reported-by: Euler Robot
>> Signed-off-by: Alex Chen
>
> Your From: line ("AlexChen") is spelled differently than your S-o-b:
> line ("Alex Chen"). While this is not fatal to the patch, it is
> confusing, so you may want to update your git settings to produce mail
> spelled in the same manner as the S-o-b.
>
Hi Eric,
Thanks for you suggestion, I will modify the user.name of git to "Alex Chen".
> Also, although you did manage to send a 0/2 letter, you did not thread
> things:
> 0/2 Message-ID: <5fa4ae0b.1000...@huawei.com>
> 1/2 Message-ID: Message-ID: <5fa4ae11.6060...@huawei.com>, but no
> In-Reply-To: or References: headers, which means it is a new top-level
> thread. You may want to figure out why your mail setup is not
> preserving threading.
>
This may be my email settings is wrong, I try to modify the setting and send a
patch v3.
Thanks,
Alex
Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
On 2020/11/6 22:16, Philippe Mathieu-Daudé wrote:
> On 11/3/20 8:46 AM, AlexChen wrote:
>> The size of env->mmu.regs is 3, but the range of 'rn' is [0, 5].
>> To avoid data access out of bounds, only if 'rn' is less than 3, we
>> can print env->mmu.regs[rn]. In other cases, we can print
>> env->mmu.regs[MMU_R_TLBX].
>>
>> Reported-by: Euler Robot
>> Signed-off-by: Alex Chen
>> ---
>> target/microblaze/mmu.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
>> index 1dbbb271c4..917ad6d69e 100644
>> --- a/target/microblaze/mmu.c
>> +++ b/target/microblaze/mmu.c
>> @@ -234,7 +234,8 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn,
>> uint32_t v)
>> unsigned int i;
>>
>> qemu_log_mask(CPU_LOG_MMU,
>> - "%s rn=%d=%x old=%x\n", __func__, rn, v,
>> env->mmu.regs[rn]);
>> + "%s rn=%d=%x old=%x\n", __func__, rn, v,
>> + rn < 3 ? env->mmu.regs[rn] : env->mmu.regs[MMU_R_TLBX]);
>
> Nack. If rn >= ARRAY_SIZE(env->mmu.regs), then don't displays it.
> Else it is confuse to see a value unrelated to the MMU index used...
>
Hi Philippe,
Thanks for your review.
The env->mmu.regs[MMU_R_TLBX] is used when rn >= ARRAY_SIZE(env->mmu.regs),
can we change the description of the log as follows so that it doesn't confuse
us?
---
target/microblaze/mmu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index 1dbbb271c4..14863ed8d1 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -234,7 +234,9 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn,
uint32_t v)
unsigned int i;
qemu_log_mask(CPU_LOG_MMU,
- "%s rn=%d=%x old=%x\n", __func__, rn, v, env->mmu.regs[rn]);
+ "%s rn=%d=%x %s=%x\n", __func__, rn, v,
+ rn < 3 ? "old" : "regs[MMU_R_TLBX]",
+ rn < 3 ? env->mmu.regs[rn] : env->mmu.regs[MMU_R_TLBX]);
if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
Thanks,
Alex
[PATCH V2 2/2] plugins: Fix two resource leaks in setup_socket()
Either accept() fails or exits normally, we need to close the fd.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
contrib/plugins/lockstep.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 319bd44b83..5aad50869d 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
socket_fd = accept(fd, NULL, NULL);
if (socket_fd < 0 && errno != EINTR) {
perror("accept socket");
+close(fd);
return false;
}
qemu_plugin_outs("setup_socket::ready\n");
+close(fd);
return true;
}
--
2.19.1
[PATCH V2 1/2] plugins: Fix resource leak in connect_socket()
Close the fd when the connect() fails.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
contrib/plugins/lockstep.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index a696673dff..319bd44b83 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -292,6 +292,7 @@ static bool connect_socket(const char *path)
if (connect(fd, (struct sockaddr *), sizeof(sockaddr)) < 0) {
perror("failed to connect");
+close(fd);
return false;
}
--
2.19.1
[PATCH V2 0/2] plugins: Fix some resource leaks
There are 3 resource leaks in contrib/plugins/lockstep.c, fix it. v1->v2: - add the cover letter - modify the subject of the patch[2/2] alexchen (2): plugins: Fix resource leak in connect_socket() plugins: Fix two resource leaks in setup_socket() contrib/plugins/lockstep.c | 3 +++ 1 file changed, 3 insertions(+) -- 2.19.1
Re: [PATCH 1/2] plugins: Fix resource leak in connect_socket()
On 2020/11/5 18:37, Alex Bennée wrote: > > AlexChen writes: > >> Kindly ping. > > Ahh sorry I missed these. Was there a cover letter for the series? > I forgot to send the cover letter, I will send the patch V2 with the cover letter. Thanks, Alex Chen
[PATCH] tests/qtest/tpm: Remove redundant check in the tpm_test_swtpm_test()
The 'addr' would not be NULL after checking 'succ' is valid,
and it has been dereferenced in the previous code(args = g_strdup_printf()).
So the check on 'addr' in the tpm_test_swtpm_test() is redundant. Remove it.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
tests/qtest/tpm-tests.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/tpm-tests.c b/tests/qtest/tpm-tests.c
index 70c80f8379..0da3a8a4df 100644
--- a/tests/qtest/tpm-tests.c
+++ b/tests/qtest/tpm-tests.c
@@ -70,10 +70,8 @@ void tpm_test_swtpm_test(const char *src_tpm_path, tx_func
*tx,
qtest_end();
tpm_util_swtpm_kill(swtpm_pid);
-if (addr) {
-g_unlink(addr->u.q_unix.path);
-qapi_free_SocketAddress(addr);
-}
+g_unlink(addr->u.q_unix.path);
+qapi_free_SocketAddress(addr);
}
void tpm_test_swtpm_migration_test(const char *src_tpm_path,
--
2.19.1
[PATCH] usb/hcd-xhci: Fix an index-out-of-bounds in xhci_runtime_write and xhci_runtime_read
Currently, the 'v' is not checked whether it is between 0 and 16,
which may result in an out-of-bounds access to the array 'xhci->intr[]'.
This is LP#1902112. Following is the reproducer provided in:
-->https://bugs.launchpad.net/qemu/+bug/1902112
=== Reproducer (build with --enable-sanitizers) ===
export UBSAN_OPTIONS="print_stacktrace=1:silence_unsigned_overflow=1"
cat << EOF | ./qemu-system-i386 -display none -machine\
accel=qtest, -m 512M -machine q35 -nodefaults -drive\
file=null-co://,if=none,format=raw,id=disk0 -device\
qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0\
-device usb-bot -device usb-storage,drive=disk0\
-chardev null,id=cd0 -chardev null,id=cd1 -device\
usb-braille,chardev=cd0 -device usb-ccid -device\
usb-ccid -device usb-kbd -device usb-mouse -device\
usb-serial,chardev=cd1 -device usb-tablet -device\
usb-wacom-tablet -device usb-audio -qtest stdio
outl 0xcf8 0x8803
outl 0xcfc 0x18ca
outl 0xcf8 0x8810
outl 0xcfc 0x555a2e46
write 0x555a1004 0x4 0xe7b9aa7a
EOF
=== Stack Trace ===
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../hw/usb/hcd-xhci.c:3012:30 in
../hw/usb/hcd-xhci.c:3012:30: runtime error: index -1 out of bounds for type
'XHCIInterrupter [16]'
#0 0x55bd2e97c8b0 in xhci_runtime_write /src/qemu/hw/usb/hcd-xhci.c:3012:30
#1 0x55bd2edfdd13 in memory_region_write_accessor
/src/qemu/softmmu/memory.c:484:5
#2 0x55bd2edfdb14 in access_with_adjusted_size /src/qemu/softmmu/memory.c:545:18
#3 0x55bd2edfd54b in memory_region_dispatch_write
/src/qemu/softmmu/memory.c:0:13
#4 0x55bd2ed7fa46 in flatview_write_continue /src/qemu/softmmu/physmem.c:2767:23
#5 0x55bd2ed7cac0 in flatview_write /src/qemu/softmmu/physmem.c:2807:14
This patch fixes this bug.
Buglink: https://bugs.launchpad.net/qemu/+bug/1902112
Reported-by: Alexander Bulekov
Signed-off-by: Alex Chen
---
hw/usb/hcd-xhci.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 79ce5c4be6..50abef40ad 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2962,8 +2962,9 @@ static uint64_t xhci_runtime_read(void *ptr, hwaddr reg,
{
XHCIState *xhci = ptr;
uint32_t ret = 0;
+int v = (reg - 0x20) / 0x20;
-if (reg < 0x20) {
+if (reg < 0x20 || v < 0 || v >= XHCI_MAXINTRS) {
switch (reg) {
case 0x00: /* MFINDEX */
ret = xhci_mfindex_get(xhci) & 0x3fff;
@@ -2973,7 +2974,6 @@ static uint64_t xhci_runtime_read(void *ptr, hwaddr reg,
break;
}
} else {
-int v = (reg - 0x20) / 0x20;
XHCIInterrupter *intr = >intr[v];
switch (reg & 0x1f) {
case 0x00: /* IMAN */
@@ -3009,14 +3009,16 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
{
XHCIState *xhci = ptr;
int v = (reg - 0x20) / 0x20;
-XHCIInterrupter *intr = >intr[v];
+XHCIInterrupter *intr;
trace_usb_xhci_runtime_write(reg, val);
-if (reg < 0x20) {
+if (reg < 0x20 || v < 0 || v >= XHCI_MAXINTRS) {
trace_usb_xhci_unimplemented("runtime write", reg);
return;
}
+intr = >intr[v];
+
switch (reg & 0x1f) {
case 0x00: /* IMAN */
if (val & IMAN_IP) {
--
2.19.1
Re: [PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
Kindly ping.
On 2020/10/28 21:45, AlexChen wrote:
> Either accept() fails or exits normally, we need to close the fd.
>
> Reported-by: Euler Robot
> Signed-off-by: AlexChen
> ---
> contrib/plugins/lockstep.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
> index 319bd44b83..5aad50869d 100644
> --- a/contrib/plugins/lockstep.c
> +++ b/contrib/plugins/lockstep.c
> @@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
> socket_fd = accept(fd, NULL, NULL);
> if (socket_fd < 0 && errno != EINTR) {
> perror("accept socket");
> +close(fd);
> return false;
> }
>
> qemu_plugin_outs("setup_socket::ready\n");
>
> +close(fd);
> return true;
> }
>
Re: [PATCH 1/2] plugins: Fix resource leak in connect_socket()
Kindly ping.
On 2020/10/28 21:45, AlexChen wrote:
> Close the fd when connect() fails.
>
> Reported-by: Euler Robot
> Signed-off-by: AlexChen
> ---
> contrib/plugins/lockstep.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
> index a696673dff..319bd44b83 100644
> --- a/contrib/plugins/lockstep.c
> +++ b/contrib/plugins/lockstep.c
> @@ -292,6 +292,7 @@ static bool connect_socket(const char *path)
>
> if (connect(fd, (struct sockaddr *), sizeof(sockaddr)) < 0) {
> perror("failed to connect");
> +close(fd);
> return false;
> }
>
[PATCH V2] qtest: Fix bad printf format specifiers
We should use printf format specifier PRIu32 instead of "%d" for
argument of type 'uint32_t'.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
tests/qtest/arm-cpu-features.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index d20094d5a7..003ba24fac 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -536,7 +536,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
if (kvm_supports_sve) {
g_assert(vls != 0);
max_vq = 64 - __builtin_clzll(vls);
-sprintf(max_name, "sve%d", max_vq * 128);
+sprintf(max_name, "sve%" PRIu32, max_vq * 128);
/* Enabling a supported length is of course fine. */
assert_sve_vls(qts, "host", vls, "{ %s: true }", max_name);
@@ -556,7 +556,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
* unless all larger, supported vector lengths are also
* disabled.
*/
-sprintf(name, "sve%d", vq * 128);
+sprintf(name, "sve%" PRIu32, vq * 128);
error = g_strdup_printf("cannot disable %s", name);
assert_error(qts, "host", error,
"{ %s: true, %s: false }",
@@ -569,7 +569,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
* we need at least one vector length enabled.
*/
vq = __builtin_ffsll(vls);
-sprintf(name, "sve%d", vq * 128);
+sprintf(name, "sve%" PRIu32, vq * 128);
error = g_strdup_printf("cannot disable %s", name);
assert_error(qts, "host", error, "{ %s: false }", name);
g_free(error);
@@ -581,7 +581,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
}
}
if (vq <= SVE_MAX_VQ) {
-sprintf(name, "sve%d", vq * 128);
+sprintf(name, "sve%" PRIu32, vq * 128);
error = g_strdup_printf("cannot enable %s", name);
assert_error(qts, "host", error, "{ %s: true }", name);
g_free(error);
--
2.19.1
Re: [PATCH] qtest: Fix bad printf format specifiers
On 2020/11/4 18:44, Thomas Huth wrote:
> On 04/11/2020 11.23, AlexChen wrote:
>> We should use printf format specifier "%u" instead of "%d" for
>> argument of type "unsigned int".
>>
>> Reported-by: Euler Robot
>> Signed-off-by: Alex Chen
>> ---
>> tests/qtest/arm-cpu-features.c | 8
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
>> index d20094d5a7..bc681a95d5 100644
>> --- a/tests/qtest/arm-cpu-features.c
>> +++ b/tests/qtest/arm-cpu-features.c
>> @@ -536,7 +536,7 @@ static void test_query_cpu_model_expansion_kvm(const
>> void *data)
>> if (kvm_supports_sve) {
>> g_assert(vls != 0);
>> max_vq = 64 - __builtin_clzll(vls);
>> -sprintf(max_name, "sve%d", max_vq * 128);
>> +sprintf(max_name, "sve%u", max_vq * 128);
>>
>> /* Enabling a supported length is of course fine. */
>> assert_sve_vls(qts, "host", vls, "{ %s: true }", max_name);
>> @@ -556,7 +556,7 @@ static void test_query_cpu_model_expansion_kvm(const
>> void *data)
>> * unless all larger, supported vector lengths are also
>> * disabled.
>> */
>> -sprintf(name, "sve%d", vq * 128);
>> +sprintf(name, "sve%u", vq * 128);
>> error = g_strdup_printf("cannot disable %s", name);
>> assert_error(qts, "host", error,
>> "{ %s: true, %s: false }",
>> @@ -569,7 +569,7 @@ static void test_query_cpu_model_expansion_kvm(const
>> void *data)
>> * we need at least one vector length enabled.
>> */
>> vq = __builtin_ffsll(vls);
>> -sprintf(name, "sve%d", vq * 128);
>> +sprintf(name, "sve%u", vq * 128);
>> error = g_strdup_printf("cannot disable %s", name);
>> assert_error(qts, "host", error, "{ %s: false }", name);
>> g_free(error);
>> @@ -581,7 +581,7 @@ static void test_query_cpu_model_expansion_kvm(const
>> void *data)
>> }
>> }
>> if (vq <= SVE_MAX_VQ) {
>> -sprintf(name, "sve%d", vq * 128);
>> +sprintf(name, "sve%u", vq * 128);
>> error = g_strdup_printf("cannot enable %s", name);
>> assert_error(qts, "host", error, "{ %s: true }", name);
>> g_free(error);
>>
>
> max_vq and vq are both "uint32_t" and not "unsigned int" ... so if you want
> to fix this really really correctly, please use PRIu32 from inttypes.h
> instead.
>
Hi Thomas,
Thanks for your review.
According to the definition of the macro PRIu32(# define PRIu32 "u"),
using PRIu32 works the same as using %u to print, and using PRIu32 to print
is relatively rare in QEMU(%u 720, PRIu32 only 120). Can we continue to use %u
to
print max_vq and vq in this patch.
Of course, this is just my small small suggestion. If you think it is better to
use
PRIu32 for printing, I will send patch V2.
Looking forward to your reply.
Thanks,
Alex
[PATCH] contrib/libvhost-user: Fix bad printf format specifiers
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
contrib/libvhost-user/libvhost-user.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/contrib/libvhost-user/libvhost-user.c
b/contrib/libvhost-user/libvhost-user.c
index bfec8a881a..5c73ffdd6b 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -701,7 +701,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
return false;
}
-DPRINT("Adding region: %d\n", dev->nregions);
+DPRINT("Adding region: %u\n", dev->nregions);
DPRINT("guest_phys_addr: 0x%016"PRIx64"\n",
msg_region->guest_phys_addr);
DPRINT("memory_size: 0x%016"PRIx64"\n",
@@ -848,7 +848,7 @@ vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg
*vmsg)
VhostUserMemory m = vmsg->payload.memory, *memory =
dev->nregions = memory->nregions;
-DPRINT("Nregions: %d\n", memory->nregions);
+DPRINT("Nregions: %u\n", memory->nregions);
for (i = 0; i < dev->nregions; i++) {
void *mmap_addr;
VhostUserMemoryRegion *msg_region = >regions[i];
@@ -938,7 +938,7 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
return vu_set_mem_table_exec_postcopy(dev, vmsg);
}
-DPRINT("Nregions: %d\n", memory->nregions);
+DPRINT("Nregions: %u\n", memory->nregions);
for (i = 0; i < dev->nregions; i++) {
void *mmap_addr;
VhostUserMemoryRegion *msg_region = >regions[i];
@@ -1049,8 +1049,8 @@ vu_set_vring_num_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int num = vmsg->payload.state.num;
-DPRINT("State.index: %d\n", index);
-DPRINT("State.num: %d\n", num);
+DPRINT("State.index: %u\n", index);
+DPRINT("State.num: %u\n", num);
dev->vq[index].vring.num = num;
return false;
@@ -1105,8 +1105,8 @@ vu_set_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int num = vmsg->payload.state.num;
-DPRINT("State.index: %d\n", index);
-DPRINT("State.num: %d\n", num);
+DPRINT("State.index: %u\n", index);
+DPRINT("State.num: %u\n", num);
dev->vq[index].shadow_avail_idx = dev->vq[index].last_avail_idx = num;
return false;
@@ -1117,7 +1117,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
{
unsigned int index = vmsg->payload.state.index;
-DPRINT("State.index: %d\n", index);
+DPRINT("State.index: %u\n", index);
vmsg->payload.state.num = dev->vq[index].last_avail_idx;
vmsg->size = sizeof(vmsg->payload.state);
@@ -1478,8 +1478,8 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
unsigned int enable = vmsg->payload.state.num;
-DPRINT("State.index: %d\n", index);
-DPRINT("State.enable: %d\n", enable);
+DPRINT("State.index: %u\n", index);
+DPRINT("State.enable: %u\n", enable);
if (index >= dev->max_queues) {
vu_panic(dev, "Invalid vring_enable index: %u", index);
@@ -1728,7 +1728,7 @@ vu_handle_vring_kick(VuDev *dev, VhostUserMsg *vmsg)
return false;
}
-DPRINT("Got kick message: handler:%p idx:%d\n",
+DPRINT("Got kick message: handler:%p idx:%u\n",
dev->vq[index].handler, index);
if (!dev->vq[index].started) {
@@ -1772,7 +1772,7 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("Request: %s (%d)\n", vu_request_to_string(vmsg->request),
vmsg->request);
DPRINT("Flags: 0x%x\n", vmsg->flags);
-DPRINT("Size:%d\n", vmsg->size);
+DPRINT("Size:%u\n", vmsg->size);
if (vmsg->fd_num) {
int i;
--
2.19.1
[PATCH] qtest: Fix bad printf format specifiers
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
tests/qtest/arm-cpu-features.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index d20094d5a7..bc681a95d5 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -536,7 +536,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
if (kvm_supports_sve) {
g_assert(vls != 0);
max_vq = 64 - __builtin_clzll(vls);
-sprintf(max_name, "sve%d", max_vq * 128);
+sprintf(max_name, "sve%u", max_vq * 128);
/* Enabling a supported length is of course fine. */
assert_sve_vls(qts, "host", vls, "{ %s: true }", max_name);
@@ -556,7 +556,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
* unless all larger, supported vector lengths are also
* disabled.
*/
-sprintf(name, "sve%d", vq * 128);
+sprintf(name, "sve%u", vq * 128);
error = g_strdup_printf("cannot disable %s", name);
assert_error(qts, "host", error,
"{ %s: true, %s: false }",
@@ -569,7 +569,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
* we need at least one vector length enabled.
*/
vq = __builtin_ffsll(vls);
-sprintf(name, "sve%d", vq * 128);
+sprintf(name, "sve%u", vq * 128);
error = g_strdup_printf("cannot disable %s", name);
assert_error(qts, "host", error, "{ %s: false }", name);
g_free(error);
@@ -581,7 +581,7 @@ static void test_query_cpu_model_expansion_kvm(const void
*data)
}
}
if (vq <= SVE_MAX_VQ) {
-sprintf(name, "sve%d", vq * 128);
+sprintf(name, "sve%u", vq * 128);
error = g_strdup_printf("cannot enable %s", name);
assert_error(qts, "host", error, "{ %s: true }", name);
g_free(error);
--
2.19.1
[PATCH] ssi: Fix bad printf format specifiers
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
hw/ssi/imx_spi.c| 2 +-
hw/ssi/xilinx_spi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index 7f703d8328..d8885ae454 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -53,7 +53,7 @@ static const char *imx_spi_reg_name(uint32_t reg)
case ECSPI_MSGDATA:
return "ECSPI_MSGDATA";
default:
-sprintf(unknown, "%d ?", reg);
+sprintf(unknown, "%u ?", reg);
return unknown;
}
}
diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
index fec8817d94..49ff275593 100644
--- a/hw/ssi/xilinx_spi.c
+++ b/hw/ssi/xilinx_spi.c
@@ -142,7 +142,7 @@ static void xlx_spi_update_irq(XilinxSPI *s)
irq chain unless things really changed. */
if (pending != s->irqline) {
s->irqline = pending;
-DB_PRINT("irq_change of state %d ISR:%x IER:%X\n",
+DB_PRINT("irq_change of state %u ISR:%x IER:%X\n",
pending, s->regs[R_IPISR], s->regs[R_IPIER]);
qemu_set_irq(s->irq, pending);
}
--
2.19.1
[PATCH] tests/qtest: Fix potential NULL pointer dereference in qos_build_main_args()
In qos_build_main_args(), the pointer 'path' is dereferenced before
checking it is valid, which may lead to NULL pointer dereference.
So move the assignment to 'cmd_line' after checking 'path' is valid.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
tests/qtest/fuzz/qos_fuzz.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index b943577b8c..cee1a2a60f 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -70,7 +70,7 @@ static GString *qos_build_main_args(void)
{
char **path = fuzz_path_vec;
QOSGraphNode *test_node;
-GString *cmd_line = g_string_new(path[0]);
+GString *cmd_line;
void *test_arg;
if (!path) {
@@ -79,6 +79,7 @@ static GString *qos_build_main_args(void)
}
/* Before test */
+cmd_line = g_string_new(path[0]);
current_path = path;
test_node = qos_graph_get_node(path[(g_strv_length(path) - 1)]);
test_arg = test_node->u.test.arg;
--
2.19.1
Re: [PATCH] hw/intc: Fix incorrect calculation of core in liointc_read() and liointc_write()
On 2020/11/3 17:53, Jiaxun Yang wrote:
>
>
> 在 2020/11/3 17:32, AlexChen 写道:
>> According to the loongson spec
>> (http://www.loongson.cn/uploadfile/cpu/3B1500/Loongson_3B1500_cpu_user_1.pdf)
>> and the macro definition(#define R_PERCORE_ISR(x) (0x40 + 0x8 * x)), we know
>> that the ISR size of per CORE is 8, so here we need to divide
>> (addr - R_PERCORE_ISR(0)) by 8, not 4.
> Hi Alex
>
> Thanks!
>
> That was my fault.. Per Core ISA is rarely used by kernel..
>
> Reviewed-by: Jiaxun Yang
>> Reported-by: Euler Robot
> Btw:
> How can you discover this by robot?
> Huawei owns real artifical intelligence technology lol :-）
>
>
Thanks for your review.
EulerRobot is a virtualization software quality automation project that
integrates some tools and test suites such as gcc/clang make test, qemu ut,
qtest, coccinelle scripts and avocado-vt.
The code checking tool found there was a potential array out of bounds at
'r = p->per_core_isr[core]', since 'core' may be 7 which is bigger than
'per_core_isr' array size 3.
So we found this bug.
Thanks,
Alex
> - Jiaxun
>> Signed-off-by: Alex Chen
>> ---
>> hw/intc/loongson_liointc.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
>> index 30fb375b72..fbbfb57ee9 100644
>> --- a/hw/intc/loongson_liointc.c
>> +++ b/hw/intc/loongson_liointc.c
>> @@ -130,7 +130,7 @@ liointc_read(void *opaque, hwaddr addr, unsigned int
>> size)
>>
>> if (addr >= R_PERCORE_ISR(0) &&
>> addr < R_PERCORE_ISR(NUM_CORES)) {
>> -int core = (addr - R_PERCORE_ISR(0)) / 4;
>> +int core = (addr - R_PERCORE_ISR(0)) / 8;
>> r = p->per_core_isr[core];
>> goto out;
>> }
>> @@ -173,7 +173,7 @@ liointc_write(void *opaque, hwaddr addr,
>>
>> if (addr >= R_PERCORE_ISR(0) &&
>> addr < R_PERCORE_ISR(NUM_CORES)) {
>> -int core = (addr - R_PERCORE_ISR(0)) / 4;
>> +int core = (addr - R_PERCORE_ISR(0)) / 8;
>> p->per_core_isr[core] = value;
>> goto out;
>> }
> .
>
Re: block: Remove unused include
On 2020/11/3 21:26, Max Reitz wrote: > On 21.10.20 11:12, AlexChen wrote: >> The "qemu-common.h" include is not used, remove it. >> >> Reported-by: Euler Robot >> Signed-off-by: AlexChen >> --- >> block/dmg-lzfse.c | 1 - >> 1 file changed, 1 deletion(-) > > Thanks, I’ve applied this patch to my block branch: > > https://git.xanclic.moe/XanClic/qemu/commits/branch/block > > Please note (for future patches) that all patches’ subjects should by > prefixed by “[PATCH]”, as done by git format-patch (see > https://wiki.qemu.org/Contribute/SubmitAPatch#Use_git_format-patch). > Oh, this is my fault, I will pay attention to this in my subsequent patches. Thanks, Alex
[PATCH V2] block/vvfat: Fix bad printf format specifiers
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
In addition, fix two error format problems found by checkpatch.pl:
ERROR: space required after that ',' (ctx:VxV)
+fprintf(stderr,"%s attributes=0x%02x begin=%u size=%d\n",
^
ERROR: line over 90 characters
+fprintf(stderr, "%d, %s (%u, %d)\n", i, commit->path ? commit->path :
"(null)", commit->param.rename.cluster, commit->action);
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
block/vvfat.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 5abb90e7c7..54807f82ca 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1437,7 +1437,7 @@ static void print_direntry(const direntry_t* direntry)
for(i=0;i<11;i++)
ADD_CHAR(direntry->name[i]);
buffer[j] = 0;
-fprintf(stderr,"%s attributes=0x%02x begin=%d size=%d\n",
+fprintf(stderr, "%s attributes=0x%02x begin=%u size=%u\n",
buffer,
direntry->attributes,
begin_of_direntry(direntry),le32_to_cpu(direntry->size));
@@ -1446,7 +1446,7 @@ static void print_direntry(const direntry_t* direntry)
static void print_mapping(const mapping_t* mapping)
{
-fprintf(stderr, "mapping (%p): begin, end = %d, %d, dir_index = %d, "
+fprintf(stderr, "mapping (%p): begin, end = %u, %u, dir_index = %u, "
"first_mapping_index = %d, name = %s, mode = 0x%x, " ,
mapping, mapping->begin, mapping->end, mapping->dir_index,
mapping->first_mapping_index, mapping->path, mapping->mode);
@@ -1454,7 +1454,7 @@ static void print_mapping(const mapping_t* mapping)
if (mapping->mode & MODE_DIRECTORY)
fprintf(stderr, "parent_mapping_index = %d, first_dir_index = %d\n",
mapping->info.dir.parent_mapping_index, mapping->info.dir.first_dir_index);
else
-fprintf(stderr, "offset = %d\n", mapping->info.file.offset);
+fprintf(stderr, "offset = %u\n", mapping->info.file.offset);
}
#endif
@@ -1588,7 +1588,7 @@ typedef struct commit_t {
static void clear_commits(BDRVVVFATState* s)
{
int i;
-DLOG(fprintf(stderr, "clear_commits (%d commits)\n", s->commits.next));
+DLOG(fprintf(stderr, "clear_commits (%u commits)\n", s->commits.next));
for (i = 0; i < s->commits.next; i++) {
commit_t* commit = array_get(&(s->commits), i);
assert(commit->path || commit->action == ACTION_WRITEOUT);
@@ -2648,7 +2648,9 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
fprintf(stderr, "handle_renames\n");
for (i = 0; i < s->commits.next; i++) {
commit_t* commit = array_get(&(s->commits), i);
-fprintf(stderr, "%d, %s (%d, %d)\n", i, commit->path ? commit->path :
"(null)", commit->param.rename.cluster, commit->action);
+fprintf(stderr, "%d, %s (%u, %d)\n", i,
+commit->path ? commit->path : "(null)",
+commit->param.rename.cluster, commit->action);
}
#endif
--
2.19.1
Re: [PATCH] block/vvfat: Fix bad printf format specifiers
On 2020/11/3 17:30, Kevin Wolf wrote: > Am 02.11.2020 um 12:52 hat AlexChen geschrieben: >> We should use printf format specifier "%u" instead of "%d" for >> argument of type "unsigned int". >> In addition, fix two error format problems found by checkpatch.pl: >> ERROR: space required after that ',' (ctx:VxV) >> +fprintf(stderr,"%s attributes=0x%02x begin=%u size=%d\n", >>^ >> ERROR: line over 90 characters >> +fprintf(stderr, "%d, %s (%u, %d)\n", i, commit->path ? commit->path >> : "(null)", commit->param.rename.cluster, commit->action); >> >> Reported-by: Euler Robot >> Signed-off-by: Alex Chen >> --- >> block/vvfat.c | 12 +++- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/block/vvfat.c b/block/vvfat.c >> index 5abb90e7c7..cc2ec9af21 100644 >> --- a/block/vvfat.c >> +++ b/block/vvfat.c >> @@ -1437,7 +1437,7 @@ static void print_direntry(const direntry_t* direntry) >> for(i=0;i<11;i++) >> ADD_CHAR(direntry->name[i]); >> buffer[j] = 0; >> -fprintf(stderr,"%s attributes=0x%02x begin=%d size=%d\n", >> +fprintf(stderr, "%s attributes=0x%02x begin=%u size=%d\n", >> buffer, >> direntry->attributes, >> begin_of_direntry(direntry),le32_to_cpu(direntry->size)); > > direntry->size is unsigned, too, so if we want to fix this, I think we > should fix both specifiers. > > The rest of the patch looks good. > Thanks for your review, I will fix it in my patch V2. Thanks, Alex
[PATCH] hw/intc: Fix incorrect calculation of core in liointc_read() and liointc_write()
According to the loongson spec
(http://www.loongson.cn/uploadfile/cpu/3B1500/Loongson_3B1500_cpu_user_1.pdf)
and the macro definition(#define R_PERCORE_ISR(x) (0x40 + 0x8 * x)), we know
that the ISR size of per CORE is 8, so here we need to divide
(addr - R_PERCORE_ISR(0)) by 8, not 4.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
hw/intc/loongson_liointc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
index 30fb375b72..fbbfb57ee9 100644
--- a/hw/intc/loongson_liointc.c
+++ b/hw/intc/loongson_liointc.c
@@ -130,7 +130,7 @@ liointc_read(void *opaque, hwaddr addr, unsigned int size)
if (addr >= R_PERCORE_ISR(0) &&
addr < R_PERCORE_ISR(NUM_CORES)) {
-int core = (addr - R_PERCORE_ISR(0)) / 4;
+int core = (addr - R_PERCORE_ISR(0)) / 8;
r = p->per_core_isr[core];
goto out;
}
@@ -173,7 +173,7 @@ liointc_write(void *opaque, hwaddr addr,
if (addr >= R_PERCORE_ISR(0) &&
addr < R_PERCORE_ISR(NUM_CORES)) {
-int core = (addr - R_PERCORE_ISR(0)) / 4;
+int core = (addr - R_PERCORE_ISR(0)) / 8;
p->per_core_isr[core] = value;
goto out;
}
--
2.19.1
[PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
The size of env->mmu.regs is 3, but the range of 'rn' is [0, 5].
To avoid data access out of bounds, only if 'rn' is less than 3, we
can print env->mmu.regs[rn]. In other cases, we can print
env->mmu.regs[MMU_R_TLBX].
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
target/microblaze/mmu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index 1dbbb271c4..917ad6d69e 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -234,7 +234,8 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn,
uint32_t v)
unsigned int i;
qemu_log_mask(CPU_LOG_MMU,
- "%s rn=%d=%x old=%x\n", __func__, rn, v, env->mmu.regs[rn]);
+ "%s rn=%d=%x old=%x\n", __func__, rn, v,
+ rn < 3 ? env->mmu.regs[rn] : env->mmu.regs[MMU_R_TLBX]);
if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
--
2.19.1
Re: [PATCH 0/4] qga: Fix some style problems
Kindly ping.
On 2020/10/26 17:05, AlexChen wrote:
> Fix some error style problems found by checkpatch.pl.
>
> alexchen (4):
> qga: Add spaces around operator
> qga: Delete redundant spaces
> qga: Open brace '{' following struct go on the same
> qga: switch and case should be at the same indent
>
> qga/channel-win32.c | 6 ++---
> qga/commands-posix.c | 4 +--
> qga/commands-win32.c | 28 ++---
> qga/commands.c | 4 +--
> qga/main.c | 59 ++--
> 5 files changed, 50 insertions(+), 51 deletions(-)
>
[PATCH] block/vvfat: Fix bad printf format specifiers
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
In addition, fix two error format problems found by checkpatch.pl:
ERROR: space required after that ',' (ctx:VxV)
+fprintf(stderr,"%s attributes=0x%02x begin=%u size=%d\n",
^
ERROR: line over 90 characters
+fprintf(stderr, "%d, %s (%u, %d)\n", i, commit->path ? commit->path :
"(null)", commit->param.rename.cluster, commit->action);
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
block/vvfat.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 5abb90e7c7..cc2ec9af21 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1437,7 +1437,7 @@ static void print_direntry(const direntry_t* direntry)
for(i=0;i<11;i++)
ADD_CHAR(direntry->name[i]);
buffer[j] = 0;
-fprintf(stderr,"%s attributes=0x%02x begin=%d size=%d\n",
+fprintf(stderr, "%s attributes=0x%02x begin=%u size=%d\n",
buffer,
direntry->attributes,
begin_of_direntry(direntry),le32_to_cpu(direntry->size));
@@ -1446,7 +1446,7 @@ static void print_direntry(const direntry_t* direntry)
static void print_mapping(const mapping_t* mapping)
{
-fprintf(stderr, "mapping (%p): begin, end = %d, %d, dir_index = %d, "
+fprintf(stderr, "mapping (%p): begin, end = %u, %u, dir_index = %u, "
"first_mapping_index = %d, name = %s, mode = 0x%x, " ,
mapping, mapping->begin, mapping->end, mapping->dir_index,
mapping->first_mapping_index, mapping->path, mapping->mode);
@@ -1454,7 +1454,7 @@ static void print_mapping(const mapping_t* mapping)
if (mapping->mode & MODE_DIRECTORY)
fprintf(stderr, "parent_mapping_index = %d, first_dir_index = %d\n",
mapping->info.dir.parent_mapping_index, mapping->info.dir.first_dir_index);
else
-fprintf(stderr, "offset = %d\n", mapping->info.file.offset);
+fprintf(stderr, "offset = %u\n", mapping->info.file.offset);
}
#endif
@@ -1588,7 +1588,7 @@ typedef struct commit_t {
static void clear_commits(BDRVVVFATState* s)
{
int i;
-DLOG(fprintf(stderr, "clear_commits (%d commits)\n", s->commits.next));
+DLOG(fprintf(stderr, "clear_commits (%u commits)\n", s->commits.next));
for (i = 0; i < s->commits.next; i++) {
commit_t* commit = array_get(&(s->commits), i);
assert(commit->path || commit->action == ACTION_WRITEOUT);
@@ -2648,7 +2648,9 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
fprintf(stderr, "handle_renames\n");
for (i = 0; i < s->commits.next; i++) {
commit_t* commit = array_get(&(s->commits), i);
-fprintf(stderr, "%d, %s (%d, %d)\n", i, commit->path ? commit->path :
"(null)", commit->param.rename.cluster, commit->action);
+fprintf(stderr, "%d, %s (%u, %d)\n", i,
+commit->path ? commit->path : "(null)",
+commit->param.rename.cluster, commit->action);
}
#endif
--
2.19.1
[PATCH V2] util: Remove redundant checks in the openpty()
As we can see from the following function call stack, amaster and aslave can not be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty(). In addition, according to the API specification for openpty(): https://www.gnu.org/software/libc/manual/html_node/Pseudo_002dTerminal-Pairs.html, the arguments name, termp and winp can all be NULL, but arguments amaster or aslave can not be NULL. Finally, amaster and aslave has been dereferenced at the beginning of the openpty(). So the checks on amaster and aslave in the openpty() are redundant. Remove them. Reported-by: Euler Robot Signed-off-by: Alex Chen Reviewed-by: Peter Maydell --- util/qemu-openpty.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c index eb17f5b0bc..427f43a769 100644 --- a/util/qemu-openpty.c +++ b/util/qemu-openpty.c @@ -80,10 +80,9 @@ static int openpty(int *amaster, int *aslave, char *name, (termp != NULL && tcgetattr(sfd, termp) < 0)) goto err; -if (amaster) -*amaster = mfd; -if (aslave) -*aslave = sfd; +*amaster = mfd; +*aslave = sfd; + if (winp) ioctl(sfd, TIOCSWINSZ, winp); -- 2.19.1
[PATCH V3] hw/display/exynos4210_fimd: Fix potential NULL pointer dereference
In exynos4210_fimd_update(), the pointer 's' is dereferenced before
checking it is valid, which may lead to NULL pointer dereference.
So move the assignment to global_width after checking 's' is valid.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
Reviewed-by: Philippe Mathieu-Daudé
---
hw/display/exynos4210_fimd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 4c16e1f5a0..34a960a976 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1275,12 +1275,14 @@ static void exynos4210_fimd_update(void *opaque)
bool blend = false;
uint8_t *host_fb_addr;
bool is_dirty = false;
-const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
+int global_width;
if (!s || !s->console || !s->enabled ||
surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {
return;
}
+
+global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
exynos4210_update_resolution(s);
surface = qemu_console_surface(s->console);
--
2.19.1
Re: [PATCH V2] hw/display/exynos4210_fimd: Fix potential NULL pointer dereference
On 2020/11/2 17:16, Philippe Mathieu-Daudé wrote: > On 11/2/20 5:39 AM, AlexChen wrote: >> In exynos4210_fimd_update(), the pointer s is dereferinced before > > Typo dereferinced -> dereferenced. > >> being check if it is valid, which may lead to NULL pointer dereference. >> So move the assignment to global_width after checking that the s is valid. > > Easier to read "after checking 's' is valid." > >> >> Reported-by: Euler Robot >> Signed-off-by: Alex Chen >> --- >> hw/display/exynos4210_fimd.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) > > Reviewed-by: Philippe Mathieu-Daudé > Thanks for your review, I will fix it in my patch V3. Thanks, Alex
[PATCH] hw/arm: Fix bad print format specifiers
We should use printf format specifier "%u" instead of "%i" for
argument of type "unsigned int".
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
hw/arm/pxa2xx.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/tosa.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 591776ba88..1a98f3bd5c 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -675,7 +675,7 @@ static void pxa2xx_ssp_write(void *opaque, hwaddr addr,
if (value & SSCR0_MOD)
printf("%s: Attempt to use network mode\n", __func__);
if (s->enable && SSCR0_DSS(value) < 4)
-printf("%s: Wrong data size: %i bits\n", __func__,
+printf("%s: Wrong data size: %u bits\n", __func__,
SSCR0_DSS(value));
if (!(value & SSCR0_SSE)) {
s->sssr = 0;
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 32bdeacfd3..772662f149 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -586,7 +586,7 @@ struct SpitzLCDTG {
static void spitz_bl_update(SpitzLCDTG *s)
{
if (s->bl_power && s->bl_intensity)
-zaurus_printf("LCD Backlight now at %i/63\n", s->bl_intensity);
+zaurus_printf("LCD Backlight now at %u/63\n", s->bl_intensity);
else
zaurus_printf("LCD Backlight now off\n");
}
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index fe88ed89fe..66b244aeff 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -150,7 +150,7 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
static uint32_t tosa_ssp_tansfer(SSISlave *dev, uint32_t value)
{
-fprintf(stderr, "TG: %d %02x\n", value >> 5, value & 0x1f);
+fprintf(stderr, "TG: %u %02x\n", value >> 5, value & 0x1f);
return 0;
}
--
2.19.1
Re: [PATCH] util: Remove redundant checks in the openpty()
On 2020/10/31 23:21, Peter Maydell wrote:
> On Sat, 31 Oct 2020 at 11:04, AlexChen wrote:
>>
>> As we can see from the following function call stack, the amaster and the
>> aslave
>> cannot be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty().
>> In addition, the amaster and the aslave has been dereferenced at the
>> beginning
>> of the openpty(). So the checks on amaster and aslave in the openpty() are
>> redundant.
>>
>> Reported-by: Euler Robot
>> Signed-off-by: Alex Chen
>
> This function is trying to match the BSD/glibc openpty()
> function, so the thing to check here is not QEMU's specific
> current usage but the API specification for openpty():
> https://www.gnu.org/software/libc/manual/html_node/Pseudo_002dTerminal-Pairs.html
> https://www.freebsd.org/cgi/man.cgi?query=openpty
>
> The spec says that name, termp and winp can all be
> NULL, but it doesn't say this for amaster and aslave,
> so indeed the change in this patch is the correct one.
>
>> ---
>> util/qemu-openpty.c | 7 +++
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
>> index eb17f5b0bc..427f43a769 100644
>> --- a/util/qemu-openpty.c
>> +++ b/util/qemu-openpty.c
>> @@ -80,10 +80,9 @@ static int openpty(int *amaster, int *aslave, char *name,
>> (termp != NULL && tcgetattr(sfd, termp) < 0))
>> goto err;
>>
>> -if (amaster)
>> -*amaster = mfd;
>> -if (aslave)
>> -*aslave = sfd;
>> +*amaster = mfd;
>> +*aslave = sfd;
>> +
>> if (winp)
>> ioctl(sfd, TIOCSWINSZ, winp);
>
> Reviewed-by: Peter Maydell
>
> though you might like to mention in the commit message that
> the openpty() API doesn't allow NULL amaster or aslave
> arguments.
>
Thanks for your review, I will add this description to my commit message in my
patch V2.
In addition, since the amaster and the aslave are not allow to be NULL,
do we need to check that the amaster and the aslave are NULL in the beginning
of the openpty()?
such as this modification:
diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
index eb17f5b0bc..1aadd39395 100644
--- a/util/qemu-openpty.c
+++ b/util/qemu-openpty.c
@@ -61,6 +61,9 @@ static int openpty(int *amaster, int *aslave, char *name,
const char *slave;
int mfd = -1, sfd = -1;
+if (!amaster || !aslave)
+goto err;
+
*amaster = *aslave = -1;
mfd = open("/dev/ptmx", O_RDWR | O_NOCTTY);
@@ -80,10 +83,9 @@ static int openpty(int *amaster, int *aslave, char *name,
(termp != NULL && tcgetattr(sfd, termp) < 0))
goto err;
-if (amaster)
-*amaster = mfd;
-if (aslave)
-*aslave = sfd;
+*amaster = mfd;
+*aslave = sfd;
+
if (winp)
ioctl(sfd, TIOCSWINSZ, winp);
@@ -92,7 +94,8 @@ static int openpty(int *amaster, int *aslave, char *name,
err:
if (sfd != -1)
close(sfd);
-close(mfd);
+if (mfd != -1)
+close(mfd);
return -1;
}
#endif
--
2.19.1
Thanks,
Alex
[PATCH V2] hw/display/exynos4210_fimd: Fix potential NULL pointer dereference
In exynos4210_fimd_update(), the pointer s is dereferinced before
being check if it is valid, which may lead to NULL pointer dereference.
So move the assignment to global_width after checking that the s is valid.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
hw/display/exynos4210_fimd.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 4c16e1f5a0..34a960a976 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1275,12 +1275,14 @@ static void exynos4210_fimd_update(void *opaque)
bool blend = false;
uint8_t *host_fb_addr;
bool is_dirty = false;
-const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
+int global_width;
if (!s || !s->console || !s->enabled ||
surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {
return;
}
+
+global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
exynos4210_update_resolution(s);
surface = qemu_console_surface(s->console);
--
2.19.1
[PATCH] util: Remove redundant checks in the openpty()
As we can see from the following function call stack, the amaster and the aslave cannot be NULL: char_pty_open() -> qemu_openpty_raw() -> openpty(). In addition, the amaster and the aslave has been dereferenced at the beginning of the openpty(). So the checks on amaster and aslave in the openpty() are redundant. Reported-by: Euler Robot Signed-off-by: Alex Chen --- util/qemu-openpty.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c index eb17f5b0bc..427f43a769 100644 --- a/util/qemu-openpty.c +++ b/util/qemu-openpty.c @@ -80,10 +80,9 @@ static int openpty(int *amaster, int *aslave, char *name, (termp != NULL && tcgetattr(sfd, termp) < 0)) goto err; -if (amaster) -*amaster = mfd; -if (aslave) -*aslave = sfd; +*amaster = mfd; +*aslave = sfd; + if (winp) ioctl(sfd, TIOCSWINSZ, winp); -- 2.19.1
[PATCH V2] hw/display/omap_lcdc: Fix potential NULL pointer dereference
In omap_lcd_interrupts(), the pointer omap_lcd is dereferinced before
being check if it is valid, which may lead to NULL pointer dereference.
So move the assignment to surface after checking that the omap_lcd is valid
and move surface_bits_per_pixel(surface) to after the surface assignment.
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
hw/display/omap_lcdc.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
index fa4a381db6..58e659c94f 100644
--- a/hw/display/omap_lcdc.c
+++ b/hw/display/omap_lcdc.c
@@ -78,14 +78,18 @@ static void omap_lcd_interrupts(struct omap_lcd_panel_s *s)
static void omap_update_display(void *opaque)
{
struct omap_lcd_panel_s *omap_lcd = (struct omap_lcd_panel_s *) opaque;
-DisplaySurface *surface = qemu_console_surface(omap_lcd->con);
+DisplaySurface *surface;
draw_line_func draw_line;
int size, height, first, last;
int width, linesize, step, bpp, frame_offset;
hwaddr frame_base;
-if (!omap_lcd || omap_lcd->plm == 1 || !omap_lcd->enable ||
-!surface_bits_per_pixel(surface)) {
+if (!omap_lcd || omap_lcd->plm == 1 || !omap_lcd->enable) {
+return;
+}
+
+surface = qemu_console_surface(omap_lcd->con);
+if (!surface_bits_per_pixel(surface)) {
return;
}
--
2.19.1
Re: [PATCH] hw/display/omap_lcdc: Fix potential NULL pointer dereference
On 2020/10/30 22:35, Peter Maydell wrote:
> On Fri, 30 Oct 2020 at 14:29, Peter Maydell wrote:
>>
>> On Fri, 30 Oct 2020 at 10:23, AlexChen wrote:
>>>
>>> In omap_lcd_interrupts(), the pointer omap_lcd is dereferenced before
>>> being check if it is valid, which may lead to NULL pointer dereference.
>>> So move the assignment to surface after checking that the omap_lcd is valid.
>>>
>>> Reported-by: Euler Robot
>>> Signed-off-by: Alex Chen
>>> ---
>>> hw/display/omap_lcdc.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>> Applied to target-arm.next, thanks.
>
> Whoops, spoke too soon. This doesn't compile:
>
> ../../hw/display/omap_lcdc.c: In function ‘omap_update_display’:
> ../../hw/display/omap_lcdc.c:88:10: error: ‘surface’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> !surface_bits_per_pixel(surface)) {
> ^~~
>
>
> because the early exit check
> if (!omap_lcd || omap_lcd->plm == 1 || !omap_lcd->enable ||
> !surface_bits_per_pixel(surface)) {
> return;
> }
>
> uses 'surface' and this patch moves the initialization of that
> variable down below its first use.
>
Oh, I apologize for this compilation error, I will fix it in my patch v2.
Thanks,
Alex
Re: [PATCH] hw/display/exynos4210_fimd: Fix potential NULL pointer dereference
On 2020/10/30 22:28, Peter Maydell wrote:
> On Fri, 30 Oct 2020 at 10:23, AlexChen wrote:
>>
>> In exynos4210_fimd_update(), the pointer s is dereferenced before
>> being check if it is valid, which may lead to NULL pointer dereference.
>> So move the assignment to global_width after checking that the s is valid
>>
>> Reported-by: Euler Robot
>> Signed-off-by: Alex Chen
>> ---
>> hw/display/exynos4210_fimd.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
>> index 4c16e1f5a0..a1179d2f89 100644
>> --- a/hw/display/exynos4210_fimd.c
>> +++ b/hw/display/exynos4210_fimd.c
>> @@ -1275,12 +1275,12 @@ static void exynos4210_fimd_update(void *opaque)
>> bool blend = false;
>> uint8_t *host_fb_addr;
>> bool is_dirty = false;
>> -const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
>>
>> if (!s || !s->console || !s->enabled ||
>> surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {
>> return;
>> }
>> +const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
>> exynos4210_update_resolution(s);
>> surface = qemu_console_surface(s->console);
>
> Yep, this is a bug, but QEMU's coding style doesn't permit
> variable declarations in the middle of functions. You need
> to split the declaration (which stays where it is) and the
> initialization (which can moved down below the !s test.
>
Thans for your review. I have also considered this modification to be
incompatible with
the QEMU's coding style. But the type of global_width is const int which cannot
be
assigned once they are defined.
Could we define the global_width as int, such as this modification:
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 4c16e1f5a0..34a960a976 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1275,12 +1275,14 @@ static void exynos4210_fimd_update(void *opaque)
bool blend = false;
uint8_t *host_fb_addr;
bool is_dirty = false;
-const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
+int global_width;
if (!s || !s->console || !s->enabled ||
surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {
return;
}
+
+global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
exynos4210_update_resolution(s);
surface = qemu_console_surface(s->console);
[PATCH] hw/display/omap_lcdc: Fix potential NULL pointer dereference
In omap_lcd_interrupts(), the pointer omap_lcd is dereferenced before
being check if it is valid, which may lead to NULL pointer dereference.
So move the assignment to surface after checking that the omap_lcd is valid.
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
hw/display/omap_lcdc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
index fa4a381db6..2941c5c67c 100644
--- a/hw/display/omap_lcdc.c
+++ b/hw/display/omap_lcdc.c
@@ -78,7 +78,7 @@ static void omap_lcd_interrupts(struct omap_lcd_panel_s *s)
static void omap_update_display(void *opaque)
{
struct omap_lcd_panel_s *omap_lcd = (struct omap_lcd_panel_s *) opaque;
-DisplaySurface *surface = qemu_console_surface(omap_lcd->con);
+DisplaySurface *surface;
draw_line_func draw_line;
int size, height, first, last;
int width, linesize, step, bpp, frame_offset;
@@ -89,6 +89,7 @@ static void omap_update_display(void *opaque)
return;
}
+surface = qemu_console_surface(omap_lcd->con);
frame_offset = 0;
if (omap_lcd->plm != 2) {
cpu_physical_memory_read(
--
2.19.1
[PATCH] hw/display/exynos4210_fimd: Fix potential NULL pointer dereference
In exynos4210_fimd_update(), the pointer s is dereferenced before
being check if it is valid, which may lead to NULL pointer dereference.
So move the assignment to global_width after checking that the s is valid
Reported-by: Euler Robot
Signed-off-by: Alex Chen
---
hw/display/exynos4210_fimd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 4c16e1f5a0..a1179d2f89 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1275,12 +1275,12 @@ static void exynos4210_fimd_update(void *opaque)
bool blend = false;
uint8_t *host_fb_addr;
bool is_dirty = false;
-const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
if (!s || !s->console || !s->enabled ||
surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) {
return;
}
+const int global_width = (s->vidtcon[2] & FIMD_VIDTCON2_SIZE_MASK) + 1;
exynos4210_update_resolution(s);
surface = qemu_console_surface(s->console);
--
2.19.1
[PATCH] net/l2tpv3: Remove redundant check in net_init_l2tpv3()
The result has been checked to be NULL before, it cannot be NULL here,
so the check is redundant. Remove it.
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
net/l2tpv3.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 55fea17c0f..e4d4218db6 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -655,9 +655,8 @@ int net_init_l2tpv3(const Netdev *netdev,
error_setg(errp, "could not bind socket err=%i", errno);
goto outerr;
}
-if (result) {
-freeaddrinfo(result);
-}
+
+freeaddrinfo(result);
memset(, 0, sizeof(hints));
@@ -686,9 +685,7 @@ int net_init_l2tpv3(const Netdev *netdev,
memcpy(s->dgram_dst, result->ai_addr, result->ai_addrlen);
s->dst_size = result->ai_addrlen;
-if (result) {
-freeaddrinfo(result);
-}
+freeaddrinfo(result);
if (l2tpv3->has_counter && l2tpv3->counter) {
s->has_counter = true;
--
2.19.1
[PATCH] contrib/rdmacm-mux: Fix error condition in hash_tbl_search_fd_by_ifid()
When fd is not found according to ifid, the _hash_tbl_search_fd_by_ifid()
returns 0 and assigns the result to *fd, so We have to check that *fd is 0,
not that fd is 0.
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
contrib/rdmacm-mux/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c
index bd82abbad3..771ca01e03 100644
--- a/contrib/rdmacm-mux/main.c
+++ b/contrib/rdmacm-mux/main.c
@@ -186,7 +186,7 @@ static int hash_tbl_search_fd_by_ifid(int *fd, __be64
*gid_ifid)
*fd = _hash_tbl_search_fd_by_ifid(gid_ifid);
pthread_rwlock_unlock();
-if (!fd) {
+if (!*fd) {
syslog(LOG_WARNING, "Can't find matching for ifid 0x%llx\n",
*gid_ifid);
return -ENOENT;
}
--
2.19.1
Re: block: Remove unused include
Kindly ping. Thanks, Alex On 2020/10/21 17:12, AlexChen wrote: > The "qemu-common.h" include is not used, remove it. > > Reported-by: Euler Robot > Signed-off-by: AlexChen > --- > block/dmg-lzfse.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c > index 19d25bc646..6798cf4fbf 100644 > --- a/block/dmg-lzfse.c > +++ b/block/dmg-lzfse.c > @@ -22,7 +22,6 @@ > * THE SOFTWARE. > */ > #include "qemu/osdep.h" > -#include "qemu-common.h" > #include "dmg.h" > #include >
Re: [PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
On 2020/10/28 15:44, Paolo Bonzini wrote: > On 28/10/20 08:11, AlexChen wrote: >> The current 'DEBUG_KVM' macro is defined in many files, and turning on >> the debug switch requires code modification, which is very inconvenient, >> so this series add an option to configure to support the definition of >> the 'DEBUG_KVM' macro. >> In addition, patches 3 and 4 also make printf always compile in debug output >> which will prevent bitrot of the format strings by referring to the >> commit(08564ecd: s390x/kvm: make printf always compile in debug output). > > Mostly we should use tracepoints, but the usefulness of these printf > statements is often limited (except for s390 that maybe could make them > unconditional error_reports). I would leave this as is, maintainers can > decide which tracepoints they like to have. > Thanks for your review, I agree with you to leave 'DEBUG_KVM' as is. In addition, patches 3 and 4 resolved the potential risk of bitrot of the format strings, could you help review these two patches? Thanks, Alex
[PATCH V2] vhost-user-blk/scsi: Fix broken error handling for socket call
When socket() fails, it returns -1, 0 is the normal return value and should not
return error.
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
contrib/vhost-user-blk/vhost-user-blk.c | 2 +-
contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
b/contrib/vhost-user-blk/vhost-user-blk.c
index 25eccd02b5..40a2dfc544 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
-if (sock <= 0) {
+if (sock < 0) {
perror("socket");
return -1;
}
diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 3c912384e9..0f9ba4b2a2 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
-if (sock <= 0) {
+if (sock < 0) {
perror("socket");
return -1;
}
--
2.19.1
Re: [PATCH] vhost-user-blk: Fix two resource leaks
On 2020/10/28 23:40, Raphael Norwitz wrote:
> The change looks good but I'm not sure I'd call it resource leak in
> either case since the failure case kills vhost-user-blk/scsi. In the
> commit message maybe rather say "vhost-user-blk/scsi: fix broken error
> handling for socket call"?
>
Thanks for your suggestion. I will modify the commit message in next version.
Thanks,
Alex
> On Wed, Oct 28, 2020 at 10:10 AM AlexChen wrote:
>>
>> When socket() fails, it returns -1, 0 is the normal return value and should
>> not return
>>
>> Reported-by: Euler Robot
>> Signed-off-by: AlexChen
>> ---
>> contrib/vhost-user-blk/vhost-user-blk.c | 2 +-
>> contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
>> b/contrib/vhost-user-blk/vhost-user-blk.c
>> index 25eccd02b5..40a2dfc544 100644
>> --- a/contrib/vhost-user-blk/vhost-user-blk.c
>> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
>> @@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
>> assert(unix_fn);
>>
>> sock = socket(AF_UNIX, SOCK_STREAM, 0);
>> -if (sock <= 0) {
>> +if (sock < 0) {
>> perror("socket");
>> return -1;
>> }
>> diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c
>> b/contrib/vhost-user-scsi/vhost-user-scsi.c
>> index 3c912384e9..0f9ba4b2a2 100644
>> --- a/contrib/vhost-user-scsi/vhost-user-scsi.c
>> +++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
>> @@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
>> assert(unix_fn);
>>
>> sock = socket(AF_UNIX, SOCK_STREAM, 0);
>> -if (sock <= 0) {
>> +if (sock < 0) {
>> perror("socket");
>> return -1;
>> }
>> --
>> 2.19.1
>>
> .
>
[PATCH] vhost-user-blk: Fix two resource leaks
When socket() fails, it returns -1, 0 is the normal return value and should not
return
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
contrib/vhost-user-blk/vhost-user-blk.c | 2 +-
contrib/vhost-user-scsi/vhost-user-scsi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
b/contrib/vhost-user-blk/vhost-user-blk.c
index 25eccd02b5..40a2dfc544 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -474,7 +474,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
-if (sock <= 0) {
+if (sock < 0) {
perror("socket");
return -1;
}
diff --git a/contrib/vhost-user-scsi/vhost-user-scsi.c
b/contrib/vhost-user-scsi/vhost-user-scsi.c
index 3c912384e9..0f9ba4b2a2 100644
--- a/contrib/vhost-user-scsi/vhost-user-scsi.c
+++ b/contrib/vhost-user-scsi/vhost-user-scsi.c
@@ -320,7 +320,7 @@ static int unix_sock_new(char *unix_fn)
assert(unix_fn);
sock = socket(AF_UNIX, SOCK_STREAM, 0);
-if (sock <= 0) {
+if (sock < 0) {
perror("socket");
return -1;
}
--
2.19.1
[PATCH 2/2] plugins: Fix two resource leaks in connect_socket()
Either accept() fails or exits normally, we need to close the fd.
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
contrib/plugins/lockstep.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 319bd44b83..5aad50869d 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -268,11 +268,13 @@ static bool setup_socket(const char *path)
socket_fd = accept(fd, NULL, NULL);
if (socket_fd < 0 && errno != EINTR) {
perror("accept socket");
+close(fd);
return false;
}
qemu_plugin_outs("setup_socket::ready\n");
+close(fd);
return true;
}
--
2.19.1
[PATCH 1/2] plugins: Fix resource leak in connect_socket()
Close the fd when connect() fails.
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
contrib/plugins/lockstep.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index a696673dff..319bd44b83 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -292,6 +292,7 @@ static bool connect_socket(const char *path)
if (connect(fd, (struct sockaddr *), sizeof(sockaddr)) < 0) {
perror("failed to connect");
+close(fd);
return false;
}
--
2.19.1
[PATCH 4/4] i386/kvm: make printf always compile in debug output
Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.
Signed-off-by: AlexChen
---
target/i386/kvm.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3e9344aed5..64492cb851 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -50,14 +50,13 @@
#include "exec/memattrs.h"
#include "trace.h"
-#ifdef CONFIG_DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do { } while (0)
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
#endif
+#define DPRINTF(fmt, ...) \
+do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } }
while (0)
+
/* From arch/x86/kvm/lapic.h */
#define KVM_APIC_BUS_CYCLE_NS 1
#define KVM_APIC_BUS_FREQUENCY (10ULL / KVM_APIC_BUS_CYCLE_NS)
--
2.19.1
[PATCH 3/4] kvm: make printf always compile in debug output
Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.
Signed-off-by: AlexChen
---
accel/kvm/kvm-all.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index fc6d99a731..854b352346 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -60,14 +60,12 @@
*/
#define PAGE_SIZE qemu_real_host_page_size
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
+#endif
-#ifdef CONFIG_DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
#define DPRINTF(fmt, ...) \
-do { } while (0)
-#endif
+do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } }
while (0)
#define KVM_MSI_HASHTAB_SIZE256
--
2.19.1
[PATCH 0/4] kvm: Add a --enable-debug-kvm option to configure
The current 'DEBUG_KVM' macro is defined in many files, and turning on the debug switch requires code modification, which is very inconvenient, so this series add an option to configure to support the definition of the 'DEBUG_KVM' macro. In addition, patches 3 and 4 also make printf always compile in debug output which will prevent bitrot of the format strings by referring to the commit(08564ecd: s390x/kvm: make printf always compile in debug output). alexchen (4): configure: Add a --enable-debug-kvm option to configure kvm: replace DEBUG_KVM to CONFIG_DEBUG_KVM kvm: make printf always compile in debug output i386/kvm: make printf always compile in debug output accel/kvm/kvm-all.c | 11 --- configure | 10 ++ target/i386/kvm.c | 11 --- target/mips/kvm.c | 6 -- target/s390x/kvm.c | 6 +++--- 5 files changed, 25 insertions(+), 19 deletions(-) -- 2.19.1
[PATCH 1/4] configure: Add a --enable-debug-kvm option to configure
This patch allows CONFIG_DEBUG_KVM to be defined when passing
an option to the configure script.
Signed-off-by: AlexChen
---
configure | 10 ++
1 file changed, 10 insertions(+)
diff --git a/configure b/configure
index e6754c1e87..2cdef5be4c 100755
--- a/configure
+++ b/configure
@@ -338,6 +338,7 @@ pvrdma=""
gprof="no"
debug_tcg="no"
debug="no"
+debug_kvm="no"
sanitizers="no"
tsan="no"
fortify_source=""
@@ -1022,11 +1023,16 @@ for opt do
;;
--disable-debug-tcg) debug_tcg="no"
;;
+ --enable-debug-kvm) debug_kvm="yes"
+ ;;
+ --disable-debug-kvm) debug_kvm="no"
+ ;;
--enable-debug)
# Enable debugging options that aren't excessively noisy
debug_tcg="yes"
debug_mutex="yes"
debug="yes"
+ debug_kvm="yes"
strip_opt="no"
fortify_source="no"
;;
@@ -1735,6 +1741,7 @@ disabled with --disable-FEATURE, default is enabled if
available:
module-upgrades try to load modules from alternate paths for upgrades
debug-tcg TCG debugging (default is disabled)
debug-info debugging information
+ debug-kvm KVM debugging support (default is disabled)
sparse sparse checker
safe-stack SafeStack Stack Smash Protection. Depends on
clang/llvm >= 3.7 and requires coroutine backend ucontext.
@@ -5929,6 +5936,9 @@ fi
if test "$debug_tcg" = "yes" ; then
echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
fi
+if test "$debug_kvm" = "yes" ; then
+ echo "CONFIG_DEBUG_KVM=y" >> $config_host_mak
+fi
if test "$strip_opt" = "yes" ; then
echo "STRIP=${strip}" >> $config_host_mak
fi
--
2.19.1
[PATCH 2/4] kvm: Replace DEBUG_KVM with CONFIG_DEBUG_KVM
Now we can control the definition of DPRINTF by CONFIG_DEBUG_KVM,
so let's replace DEBUG_KVM with CONFIG_DEBUG_KVM.
Signed-off-by: AlexChen
---
accel/kvm/kvm-all.c | 3 +--
target/i386/kvm.c | 4 +---
target/mips/kvm.c | 6 --
target/s390x/kvm.c | 6 +++---
4 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9ef5daf4c5..fc6d99a731 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -60,9 +60,8 @@
*/
#define PAGE_SIZE qemu_real_host_page_size
-//#define DEBUG_KVM
-#ifdef DEBUG_KVM
+#ifdef CONFIG_DEBUG_KVM
#define DPRINTF(fmt, ...) \
do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
#else
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index cf46259534..3e9344aed5 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -50,9 +50,7 @@
#include "exec/memattrs.h"
#include "trace.h"
-//#define DEBUG_KVM
-
-#ifdef DEBUG_KVM
+#ifdef CONFIG_DEBUG_KVM
#define DPRINTF(fmt, ...) \
do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
#else
diff --git a/target/mips/kvm.c b/target/mips/kvm.c
index 72637a1e02..a0b979e6d2 100644
--- a/target/mips/kvm.c
+++ b/target/mips/kvm.c
@@ -28,10 +28,12 @@
#include "exec/memattrs.h"
#include "hw/boards.h"
-#define DEBUG_KVM 0
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
+#endif
#define DPRINTF(fmt, ...) \
-do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
+do { if (CONFIG_DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } }
while (0)
static int kvm_mips_fpu_cap;
static int kvm_mips_msa_cap;
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index f13eff688c..8bc9e1e468 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -52,12 +52,12 @@
#include "hw/s390x/s390-virtio-hcall.h"
#include "hw/s390x/pv.h"
-#ifndef DEBUG_KVM
-#define DEBUG_KVM 0
+#ifndef CONFIG_DEBUG_KVM
+#define CONFIG_DEBUG_KVM 0
#endif
#define DPRINTF(fmt, ...) do {\
-if (DEBUG_KVM) { \
+if (CONFIG_DEBUG_KVM) { \
fprintf(stderr, fmt, ## __VA_ARGS__); \
} \
} while (0)
--
2.19.1
[PATCH 0/4] qga: Fix some style problems
Fix some error style problems found by checkpatch.pl.
alexchen (4):
qga: Add spaces around operator
qga: Delete redundant spaces
qga: Open brace '{' following struct go on the same
qga: switch and case should be at the same indent
qga/channel-win32.c | 6 ++---
qga/commands-posix.c | 4 +--
qga/commands-win32.c | 28 ++---
qga/commands.c | 4 +--
qga/main.c | 59 ++--
5 files changed, 50 insertions(+), 51 deletions(-)
--
2.19.1
[PATCH 3/4] qga: Open brace '{' following struct go on the same
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
qga/main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 308ebd6581..69660d9abd 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -694,8 +694,7 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type,
LPVOID data,
DWORD ret = NO_ERROR;
GAService *service = _state->service;
-switch (ctrl)
-{
+switch (ctrl) {
case SERVICE_CONTROL_STOP:
case SERVICE_CONTROL_SHUTDOWN:
quit_handler(SIGTERM);
--
2.19.1
[PATCH 4/4] qga: Switch and case should be at the same indent
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
qga/main.c | 52 ++--
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 69660d9abd..33e510ba19 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -280,20 +280,20 @@ QEMU_HELP_BOTTOM "\n"
static const char *ga_log_level_str(GLogLevelFlags level)
{
switch (level & G_LOG_LEVEL_MASK) {
-case G_LOG_LEVEL_ERROR:
-return "error";
-case G_LOG_LEVEL_CRITICAL:
-return "critical";
-case G_LOG_LEVEL_WARNING:
-return "warning";
-case G_LOG_LEVEL_MESSAGE:
-return "message";
-case G_LOG_LEVEL_INFO:
-return "info";
-case G_LOG_LEVEL_DEBUG:
-return "debug";
-default:
-return "user";
+case G_LOG_LEVEL_ERROR:
+return "error";
+case G_LOG_LEVEL_CRITICAL:
+return "critical";
+case G_LOG_LEVEL_WARNING:
+return "warning";
+case G_LOG_LEVEL_MESSAGE:
+return "message";
+case G_LOG_LEVEL_INFO:
+return "info";
+case G_LOG_LEVEL_DEBUG:
+return "debug";
+default:
+return "user";
}
}
@@ -695,19 +695,19 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type,
LPVOID data,
GAService *service = _state->service;
switch (ctrl) {
-case SERVICE_CONTROL_STOP:
-case SERVICE_CONTROL_SHUTDOWN:
-quit_handler(SIGTERM);
-SetEvent(ga_state->wakeup_event);
-service->status.dwCurrentState = SERVICE_STOP_PENDING;
-SetServiceStatus(service->status_handle, >status);
-break;
-case SERVICE_CONTROL_DEVICEEVENT:
-handle_serial_device_events(type, data);
-break;
+case SERVICE_CONTROL_STOP:
+case SERVICE_CONTROL_SHUTDOWN:
+quit_handler(SIGTERM);
+SetEvent(ga_state->wakeup_event);
+service->status.dwCurrentState = SERVICE_STOP_PENDING;
+SetServiceStatus(service->status_handle, >status);
+break;
+case SERVICE_CONTROL_DEVICEEVENT:
+handle_serial_device_events(type, data);
+break;
-default:
-ret = ERROR_CALL_NOT_IMPLEMENTED;
+default:
+ret = ERROR_CALL_NOT_IMPLEMENTED;
}
return ret;
}
--
2.19.1
[PATCH 2/4] qga: Delete redundant spaces
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
qga/commands-win32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 2c341c7bea..de6e07f275 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1234,7 +1234,7 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error
**errp)
DWORD char_count = 0;
char *path, *out;
GError *gerr = NULL;
-gchar * argv[4];
+gchar *argv[4];
GetVolumePathNamesForVolumeNameW(guid, NULL, 0, _count);
@@ -2111,7 +2111,7 @@ static ga_win_10_0_server_t const
WIN_10_0_SERVER_VERSION_MATRIX[3] = {
static void ga_get_win_version(RTL_OSVERSIONINFOEXW *info, Error **errp)
{
-typedef NTSTATUS(WINAPI * rtl_get_version_t)(
+typedef NTSTATUS(WINAPI *rtl_get_version_t)(
RTL_OSVERSIONINFOEXW *os_version_info_ex);
info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
--
2.19.1
[PATCH 1/4] qga: Add spaces around operator
Reported-by: Euler Robot
Signed-off-by: AlexChen
---
qga/channel-win32.c | 6 +++---
qga/commands-posix.c | 4 ++--
qga/commands-win32.c | 24
qga/commands.c | 4 ++--
qga/main.c | 4 ++--
5 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 4f04868a76..21ec9268b4 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -292,9 +292,9 @@ static gboolean ga_channel_open(GAChannel *c,
GAChannelMethod method,
return false;
}
-if (method == GA_CHANNEL_ISA_SERIAL){
+if (method == GA_CHANNEL_ISA_SERIAL) {
snprintf(newpath, sizeof(newpath), ".\\%s", path);
-}else {
+} else {
g_strlcpy(newpath, path, sizeof(newpath));
}
@@ -307,7 +307,7 @@ static gboolean ga_channel_open(GAChannel *c,
GAChannelMethod method,
return false;
}
-if (method == GA_CHANNEL_ISA_SERIAL &&
!SetCommTimeouts(c->handle,)) {
+if (method == GA_CHANNEL_ISA_SERIAL && !SetCommTimeouts(c->handle,
)) {
g_autofree gchar *emsg = g_win32_error_message(GetLastError());
g_critical("error setting timeout for com port: %s", emsg);
CloseHandle(c->handle);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 3bffee99d4..d5ebb3d83c 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -110,7 +110,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode,
Error **errp)
reopen_fd_to_null(2);
execle("/sbin/shutdown", "shutdown", "-h", shutdown_flag, "+0",
- "hypervisor initiated shutdown", (char*)NULL, environ);
+ "hypervisor initiated shutdown", (char *)NULL, environ);
_exit(EXIT_FAILURE);
} else if (pid < 0) {
error_setg_errno(errp, errno, "failed to create child process");
@@ -479,7 +479,7 @@ GuestFileRead *guest_file_read_unsafe(GuestFileHandle *gfh,
gfh->state = RW_STATE_NEW;
}
-buf = g_malloc0(count+1);
+buf = g_malloc0(count + 1);
read_count = fread(buf, 1, count, fh);
if (ferror(fh)) {
error_setg_errno(errp, errno, "failed to read file");
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0c3c05484f..2c341c7bea 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -83,7 +83,7 @@ CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
(365 * (1970 - 1601) + \
(1970 - 1601) / 4 - 3))
-#define INVALID_SET_FILE_POINTER ((DWORD)-1)
+#define INVALID_SET_FILE_POINTER ((DWORD) - 1)
struct GuestFileHandle {
int64_t id;
@@ -110,15 +110,15 @@ static OpenFlags guest_file_open_modes[] = {
{"w", GENERIC_WRITE,CREATE_ALWAYS},
{"wb", GENERIC_WRITE,CREATE_ALWAYS},
{"a", FILE_GENERIC_APPEND, OPEN_ALWAYS },
-{"r+", GENERIC_WRITE|GENERIC_READ, OPEN_EXISTING},
-{"rb+", GENERIC_WRITE|GENERIC_READ, OPEN_EXISTING},
-{"r+b", GENERIC_WRITE|GENERIC_READ, OPEN_EXISTING},
-{"w+", GENERIC_WRITE|GENERIC_READ, CREATE_ALWAYS},
-{"wb+", GENERIC_WRITE|GENERIC_READ, CREATE_ALWAYS},
-{"w+b", GENERIC_WRITE|GENERIC_READ, CREATE_ALWAYS},
-{"a+", FILE_GENERIC_APPEND|GENERIC_READ, OPEN_ALWAYS },
-{"ab+", FILE_GENERIC_APPEND|GENERIC_READ, OPEN_ALWAYS },
-{"a+b", FILE_GENERIC_APPEND|GENERIC_READ, OPEN_ALWAYS }
+{"r+", GENERIC_WRITE | GENERIC_READ, OPEN_EXISTING},
+{"rb+", GENERIC_WRITE | GENERIC_READ, OPEN_EXISTING},
+{"r+b", GENERIC_WRITE | GENERIC_READ, OPEN_EXISTING},
+{"w+", GENERIC_WRITE | GENERIC_READ, CREATE_ALWAYS},
+{"wb+", GENERIC_WRITE | GENERIC_READ, CREATE_ALWAYS},
+{"w+b", GENERIC_WRITE | GENERIC_READ, CREATE_ALWAYS},
+{"a+", FILE_GENERIC_APPEND | GENERIC_READ, OPEN_ALWAYS },
+{"ab+", FILE_GENERIC_APPEND | GENERIC_READ, OPEN_ALWAYS },
+{"a+b", FILE_GENERIC_APPEND | GENERIC_READ, OPEN_ALWAYS }
};
#define debug_error(msg) do { \
@@ -280,7 +280,7 @@ static void acquire_privilege(const char *name, Error
**errp)
Error *local_err = NULL;
if (OpenProcessToken(GetCurrentProcess(),
-TOKEN_ADJUST_PRIVILEGES|TOKEN_QUERY, ))
+TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, ))
{
if (!LookupPrivilegeValue(NULL, name, [0].Luid)) {
error_setg(_err, QERR_QGA_COMMAND_FAILED,
@@ -1023,7 +1023,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char
*guid, Error **errp)
len = strlen(mnt_point);
mnt_point[len] = '\\';
-mnt_point[len+1] = 0;
+
block: Remove unused include
The "qemu-common.h" include is not used, remove it. Reported-by: Euler Robot Signed-off-by: AlexChen --- block/dmg-lzfse.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c index 19d25bc646..6798cf4fbf 100644 --- a/block/dmg-lzfse.c +++ b/block/dmg-lzfse.c @@ -22,7 +22,6 @@ * THE SOFTWARE. */ #include "qemu/osdep.h" -#include "qemu-common.h" #include "dmg.h" #include -- 2.19.1
io: Don't use '#' flag of printf format
Signed-off-by: AlexChen
---
io/channel-websock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 47a0e941d9..e94a1fcf99 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -746,7 +746,7 @@ static int
qio_channel_websock_decode_header(QIOChannelWebsock *ioc,
opcode != QIO_CHANNEL_WEBSOCK_OPCODE_CLOSE &&
opcode != QIO_CHANNEL_WEBSOCK_OPCODE_PING &&
opcode != QIO_CHANNEL_WEBSOCK_OPCODE_PONG) {
-error_setg(errp, "unsupported opcode: %#04x; only binary, close, "
+error_setg(errp, "unsupported opcode: 0x%04x; only binary, close, "
"ping, and pong websocket frames are supported",
opcode);
qio_channel_websock_write_close(
ioc, QIO_CHANNEL_WEBSOCK_STATUS_INVALID_DATA ,
--
2.19.1
Re: elf2dmp: Fix memory leak on main() error paths
Kindly ping.
On 2020/8/26 18:15, AlexChen wrote:
> From: AlexChen
>
> The 'kdgb' is allocating memory in get_kdbg(), but it is not freed
> in both fill_header() and fill_context() failed branches, fix it.
>
> Signed-off-by: AlexChen
> ---
> contrib/elf2dmp/main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index 9a2dbc2902..ac746e49e0 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -568,12 +568,12 @@ int main(int argc, char *argv[])
> if (fill_header(, , , KdDebuggerDataBlock, kdbg,
> KdVersionBlock, qemu_elf.state_nr)) {
> err = 1;
> -goto out_pdb;
> +goto out_kdbg;
> }
>
> if (fill_context(kdbg, , _elf)) {
> err = 1;
> -goto out_pdb;
> +goto out_kdbg;
> }
>
> if (write_dump(, , argv[2])) {
>
Re: oslib-posix：Fix handle fd leak in qemu_write_pidfile()
On 2020/8/26 18:18, Daniel P. Berrangé wrote: > On Wed, Aug 26, 2020 at 06:14:48PM +0800, AlexChen wrote: >> > From: alexchen >> > >> > The fd will leak when (a.st_ino == b.st_ino) is true, fix it. > That is *INTENTIONAL*. We're holding a lock on the file and the > lock exists only while the FD is open. When QEMU exists, the FD > is closed and the lock is released. There is no leak. > OK, I got it, thanks. Thanks Alex
elf2dmp: Fix memory leak on main() error paths
From: AlexChen
The 'kdgb' is allocating memory in get_kdbg(), but it is not freed
in both fill_header() and fill_context() failed branches, fix it.
Signed-off-by: AlexChen
---
contrib/elf2dmp/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 9a2dbc2902..ac746e49e0 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -568,12 +568,12 @@ int main(int argc, char *argv[])
if (fill_header(, , , KdDebuggerDataBlock, kdbg,
KdVersionBlock, qemu_elf.state_nr)) {
err = 1;
-goto out_pdb;
+goto out_kdbg;
}
if (fill_context(kdbg, , _elf)) {
err = 1;
-goto out_pdb;
+goto out_kdbg;
}
if (write_dump(, , argv[2])) {
--
2.19.1
oslib-posix：Fix handle fd leak in qemu_write_pidfile()
From: alexchen The fd will leak when (a.st_ino == b.st_ino) is true, fix it. Signed-off-by: AlexChen --- util/oslib-posix.c | 1 + 1 file changed, 1 insertion(+) diff --git a/util/oslib-posix.c b/util/oslib-posix.c index ad8001a4ad..74cf5e9c73 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -176,6 +176,7 @@ bool qemu_write_pidfile(const char *path, Error **errp) goto fail_unlink; } +close(fd); return true; fail_unlink: -- 2.19.1
