[PATCH] include: Add and use a global heap_strdupW() helper

[Michael Stefaniuc]

On 02/13/2018 10:50 AM, Alexandre Julliard wrote:
> Michael Stefaniuc <mstefani at mykolab.com> writes:
> 
>>> The current usage is clean only because we are not checking for
>>> allocation failure, but that's broken. If we add proper handling, then
>>> the NULL checks will be needed anyway.
>> I assumed you want less HeapAlloc failure handling and not more!
>> Especially as in the strdup cases the current "return NULL" seems to
>> be good enough in practice.
>> I don't remember to have ever seen a patch that adds extra error
>> handling for that case.
> 
> You could argue that strdup() should be treated as a nofail function,
> but then you should remove the second null check and let it crash on the
> memcpy, or use HEAP_GENERATE_EXCEPTIONS. I'd prefer to have an explicit
> xstrdup() function for this though, and have the regular strdup()
> require error handling
I don't mind a xstrdup() variant, remains to be seen how useful it would be.

I consider the current strdup() behavior to be superior to what you
propose as it is generic:
- One can still check for alloc errors iff it is needed.
- Or let subsequent code deal with the NULL pointer. This seems to be
the default and seems to work pretty well in practice.

The ship has sailed anyway:
wine$ git grep -i -P 'strdup(\w+)?\s*\(' origin dlls/ programs/ | wc -l
1606

Not all are call sites but even 1500 cases are way too many to figure
out manually what error to return. And how many of those will be a
"Needs tests"? A ton of work without a clear benefit.


As the strdup() work is controversial I'll skip making those generic.

bye
	michael