[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtio: vdpa: omit check return of g_malloc
From: |
Alex Bennée |
Subject: |
Re: [PATCH] virtio: vdpa: omit check return of g_malloc |
Date: |
Fri, 18 Sep 2020 14:12:16 +0100 |
User-agent: |
mu4e 1.5.5; emacs 28.0.50 |
Li Qiang <liq3ea@gmail.com> writes:
> Philippe Mathieu-Daudé <philmd@redhat.com> 于2020年8月19日周三 下午11:07写道:
>>
>> On 8/19/20 4:43 PM, Li Qiang wrote:
>> > If g_malloc fails, the application will be terminated.
>>
>> Which we don't want... better to use g_try_malloc() instead?
>
> I don't think so. If g_malloc return NULL it means a critical
> situation I think terminate the application
> is OK. Though I don't find any rule/practices the qemu code base uses
> g_malloc far more than
> g_try_malloc.
g_try_malloc is only for cases you could recover from, by either
deferring or doing something else. A straight out of memory failure is
fatal.
Arguably a bunch of the try_malloc's in the code base should be straight
mallocs. The ELF loaders load_symbols does it because I guess having the
symbols is a bonus and you could still run the program if a) there was
enough memory to run and b) your symbol table was very large.
>
> Thanks,
> Li Qiang
>
>>
>> > No need to check the return value of g_malloc.
>> >
>> > Signed-off-by: Li Qiang <liq3ea@163.com>
>> > ---
>> > hw/virtio/vhost-vdpa.c | 7 +------
>> > 1 file changed, 1 insertion(+), 6 deletions(-)
>> >
>> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> > index 4580f3efd8..403ae3ae07 100644
>> > --- a/hw/virtio/vhost-vdpa.c
>> > +++ b/hw/virtio/vhost-vdpa.c
>> > @@ -320,10 +320,8 @@ static int vhost_vdpa_set_config(struct vhost_dev
>> > *dev, const uint8_t *data,
>> > struct vhost_vdpa_config *config;
>> > int ret;
>> > unsigned long config_size = offsetof(struct vhost_vdpa_config, buf);
>> > +
>> > config = g_malloc(size + config_size);
>> > - if (config == NULL) {
>> > - return -1;
>> > - }
>> > config->off = offset;
>> > config->len = size;
>> > memcpy(config->buf, data, size);
>> > @@ -340,9 +338,6 @@ static int vhost_vdpa_get_config(struct vhost_dev
>> > *dev, uint8_t *config,
>> > int ret;
>> >
>> > v_config = g_malloc(config_len + config_size);
>> > - if (v_config == NULL) {
>> > - return -1;
>> > - }
>> > v_config->len = config_len;
>> > v_config->off = 0;
>> > ret = vhost_vdpa_call(dev, VHOST_VDPA_GET_CONFIG, v_config);
>> >
>>
--
Alex Bennée