Re: [PATCH v2 0/1] MR4152: dnsapi: Handle IP address as DNS name in DnsQuery_UTF8().

[Jinoh Kang (＠iamahuman)]

On Sat Oct 21 15:00:08 2023 +0000, Jinoh Kang wrote:
> > Regarding dropping the null check, I think the common rule in Wine is
> to check the allocations. I'd personally would agree that in many cases
> just skipping the check and crashing would make more sense than putting
> code for any handling the error just anyhow, but I am just following
> what I think is the rule here.
> Well, that rule sounds absurd and contradictory.
> 1. We should check for NULL.
> 2. Except NULL doesn't happen much in practice, so we shouldn't really
> check for NULL anyway.
> Correct me if I'm wrong, I don't think such rule (pretend to check for
> NULL) has ever been in enforcement for upstream patches.  In fact, I've
> got https://gitlab.winehq.org/wine/wine/-/merge_requests/3324 upstreamed
> before; not sure if this generalizes to this case too.
> Otherwise, is `assert()`-ing an option?
> > or, in case of miraculous getting memory back,
> This could be more probable than you think.  For example, another thread
> might momentarily allocate a large amount of memory (towards RSS/VM
> ulimit) and then free it.  If we race, we'll get a spontaneous
> allocation failure.
> > it will get an error anyway being unable to find it (both errors,
> ERROR_OUT_OF_MEMORY or what it would get, are not the correct result anyway).
> Depends on how the app handles the error; for example, it could retry on
> ERROR_OUT_OF_MEMORY but not whatever NXDOMAIN is.  Also, the behavior
> (3) has additional side effect of potentially generating a packet over a
> network, in a program that would not otherwise emit any packets (it only
> resolves "127.0.0.1").
> Granted, you're not making the rules here, and existing wine code
> doesn't strictly abide by graceful allocation failure handling either.
>  I was merely puzzled by how such confusing rule is in place.
For future reference: NULL check has been dropped in master.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4152#note_52757