Re: [PATCH] virtio: vdpa: omit check return of g_malloc

["Alex Bennée" <alex.bennee@linaro.org>]

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