diff mbox

[3/6] kvm: g_malloc() can't fail, bury dead error handling

Message ID 1422376711-31648-4-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Jan. 27, 2015, 4:38 p.m. UTC 
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 kvm-all.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Thomas Huth Jan. 27, 2015, 5:22 p.m. UTC | #1 
On Tue, 27 Jan 2015 17:38:28 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  kvm-all.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 2f21a4e..05a79c2 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2070,10 +2070,6 @@ int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
>          }
> 
>          bp = g_malloc(sizeof(struct kvm_sw_breakpoint));
> -        if (!bp) {
> -            return -ENOMEM;
> -        }
> -
>          bp->pc = addr;
>          bp->use_count = 1;
>          err = kvm_arch_insert_sw_breakpoint(cpu, bp);

I think I'd also use g_try_malloc() here instead. Looks like an error
gets reported to GDB when this function returns with an error code, so
returning -ENOMEM should be ok here, shouldn't it?

 Thomas
Markus Armbruster Jan. 28, 2015, 10:59 a.m. UTC | #2 
Thomas Huth <thuth@linux.vnet.ibm.com> writes:

> On Tue, 27 Jan 2015 17:38:28 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  kvm-all.c | 4 ----
>>  1 file changed, 4 deletions(-)
>> 
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 2f21a4e..05a79c2 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -2070,10 +2070,6 @@ int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
>>          }
>> 
>>          bp = g_malloc(sizeof(struct kvm_sw_breakpoint));
>> -        if (!bp) {
>> -            return -ENOMEM;
>> -        }
>> -
>>          bp->pc = addr;
>>          bp->use_count = 1;
>>          err = kvm_arch_insert_sw_breakpoint(cpu, bp);
>
> I think I'd also use g_try_malloc() here instead. Looks like an error
> gets reported to GDB when this function returns with an error code, so
> returning -ENOMEM should be ok here, shouldn't it?

Same rationale as for PATCH 2.  sizeof(struct kvm_sw_breakpoint) is 40
bytes on my system.
diff mbox

Patch

diff --git a/kvm-all.c b/kvm-all.c
index 2f21a4e..05a79c2 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2070,10 +2070,6 @@  int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr,
         }
 
         bp = g_malloc(sizeof(struct kvm_sw_breakpoint));
-        if (!bp) {
-            return -ENOMEM;
-        }
-
         bp->pc = addr;
         bp->use_count = 1;
         err = kvm_arch_insert_sw_breakpoint(cpu, bp);