[PATCH v3 3/9] winedbg: Generalise packet_realloc(), rename to buffer_realloc().

[Rémi Bernon]

On 11/16/21 18:20, Jinoh Kang wrote:
> On 11/17/21 01:58, Rémi Bernon wrote:
>> On 11/16/21 17:51, Jinoh Kang wrote:
>>> Make it return void * and accept size_t as the size.
>>>
>>> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
>>> ---
>>
>> Hi Jinoh,
>>
>> I think that now winegdb has been converted to PE format, you can instead convert all Heap* allocations to standard C allocations, and use realloc directly instead of the helper.
> 
> Sounds good.
> 
>>
>> Regarding the errors, maybe add some assert calls if you like, although I don't think we usually use that or HEAP_GENERATE_EXCEPTIONS flag,
> 
> You're right.
> 
>> and instead we just consider that if allocation returned NULL it should either be handled properly, or ignored and later access will cause exceptions.
> 
> For some reason, no uses of that function checks for the error condition and just assumes that allocation always succeeds.
> 

Well this is often the case when it's quite unexpected to fail and 
there's no proper error handling. An abort or assertion failure will not 
be much different from a segmentation fault in the end.

> So there seems to be three options:
> 
> 1. Don't check for allocation failure at all. This will AV the next time the packet buffer (gdbctx->{in,out}_buf) is accessed.
> 2. Gracefully handle all allocation failures; however, this would be out of scope of this patch serie.
> 3. Add a new helper called xrealloc() that terminates the process if allocation fails.
> 
> So I believe we should do (1) first, and then maybe later go (2)?
> 

If it's actually useful to handle the errors, and if there's a way to 
properly tell gdb about it, without ending up in a unstable state, then 
yes I think it would be better.

Until then just ignore the failure, yeah.

Cheers,
-- 
Rémi Bernon <rbernon at codeweavers.com>