Re: [PATCH] semihosting/uaccess.c: Replaced a malloc call with g_malloc.

[Richard Henderson <richard.henderson@linaro.org>]

On 7/27/23 08:04, Daniel P. Berrangé wrote:
> On Thu, Jul 27, 2023 at 03:56:23PM +0100, Peter Maydell wrote:
>> On Wed, 26 Jul 2023 at 16:21, Richard Henderson
>> <richard.henderson@linaro.org> wrote:
>>>
>>> On 7/26/23 02:43, Peter Maydell wrote:
>>>> (Something went wrong with the quoting in your email. I've
>>>> fixed it up.)
>>>>
>>>> On Wed, 26 Jul 2023 at 05:38, <dinglimin@cmss.chinamobile.com> wrote:
>>>>> Peter Maydell wrote:
>>>>>> The third part here, is that g_malloc() does not ever
>>>>>> fail -- it will abort() on out of memory. However
>>>>>> the code here is still handling g_malloc() returning NULL.
>>>>>> The equivalent for "we expect this might fail" (which we want
>>>>>> here, because the guest is passing us the length of memory
>>>>>> to try to allocate) is g_try_malloc().
>>>>
>>>>> g_malloc() is preferred more than g_try_* functions, which return NULL on error,
>>>>> when the size of the requested allocation  is small.
>>>>> This is because allocating few bytes should not be a problem in a healthy system.
>>>>
>>>> This is true. But in this particular case we cannot be sure
>>>> that the size of the allocation is small, because the size
>>>> is controlled by the guest. So we want g_try_malloc().
>>>
>>> And why do we want to use g_try_malloc instead of just sticking with malloc?
>>
>> The only real reason is just consistency
> 
> I think it is slightly stronger than that.
> 
> By using g_try_malloc we make it explicit that this scenario is
> expecting the allocation to fail and needs to handle that.
> 
> If we use plain 'malloc' it isn't clear whether we genuinely expect
> the allocation to fail, or someone just blindly checked malloc
> return value out of habit, because they didn't realize QEMU wants
> abort-on-OOM behaviour most of the time.

Ok, fair enough.


r~