[RFC] Cygwin libstdc++ plan (operator new/delete replacement)
Dave Korn
dave.korn.cygwin@googlemail.com
Sun Jun 28 17:35:00 GMT 2009
Christopher Faylor wrote:
> On Sun, Jun 28, 2009 at 02:47:24PM +0100, Dave Korn wrote:
>> to the linker spec. If we agree this is a reasonable approach to take, I
>> could rush out a gcc4-4.3.2-3 update to add this very quickly.
>>
>> So, would everyone be happy to do it this way?
>
> It looks ok with a couple of caveats/questions below.
>
> Would it make sense to standardize on this approach for the malloc
> functions too and remove the malloc_wrapper file?
Well... "not broke, don't fix"? That would be my preferred option anyway.
>> +__wrap__ZdaPv NOSIGFE
>> +__wrap__ZdaPvRKSt9nothrow_t NOSIGFE
>> +__wrap__ZdlPv NOSIGFE
>> +__wrap__ZdlPvRKSt9nothrow_t NOSIGFE
>> +__wrap__Znaj NOSIGFE
>> +__wrap__ZnajRKSt9nothrow_t NOSIGFE
>> +__wrap__Znwj NOSIGFE
>> +__wrap__ZnwjRKSt9nothrow_t NOSIGFE
>
> Could we add comments whereever these mangled functions are referenced
> showing what they really refer to when they are demangled?
Oh, can we do comments in the .din file? I noticed we already have
"dll_crt0__FP11per_process" in there and just did what it did; the only other
places these names are used are on prototypes, which serve as well as comments
for that purpose.
>> Index: winsup/cygwin/globals.cc
> See below.
>> - DWORD unused[7];
>> + DWORD unused[6];
>> + struct per_process_cxx_malloc *cxx_malloc;
>
> This should go before the unused rather than after.
Rightyho, will do. (Side note: are you sure it's for the best though? I
figured by allocating from the end, we wouldn't change the offset of the
unused[] array within the struct. But I guess I can't think of a situation
where that would cause anything to break that wasn't already broke).
>> if (u != NULL)
>> uwasnull = 0; /* Caller allocated space for per_process structure */
>> - else if ((newu = cygwin_internal (CW_USER_DATA)) == (DWORD) -1)
>> + else if (~0u == (DWORD) newu)
>
> I really detest the 0 == variable usage.
That's not quite what the above says.
> I know why people use it (yes,
> I really do) but I'd rather not see it in Cygwin source.
Do you mean the ordering of constant-before-variable in the == comparison,
or are you referring to comparing == 0 as opposed to using the logical not
operator (having overlooked the "~" operator)? Either way, I'll word it
however you like.
> But, the good news is that you don't really need it since we're getting
> rid of old Cygwin cruft anyway so this should be gone for 1.7.
Ah, of course, we can assume for certain that cygwin_internal (CW_USER_DATA)
won't fail on any 1.7 version! I'll remove the check-for-fail altogether when
I respin it.
>> @@ -84,6 +109,27 @@ _cygwin_crt0_common (MainFunc f, per_pro
>> u->realloc = &realloc;
>> u->calloc = &calloc;
>>
>> + /* Likewise for the C++ memory operators - if any. */
>> + if (newu && newu->cxx_malloc)
>> + {
>> + /* Inherit what we don't override. */
>> +#define CONDITIONALLY_OVERRIDE(MEMBER) \
>> + if (!__cygwin_cxx_malloc.MEMBER) \
>> + __cygwin_cxx_malloc.MEMBER = newu->cxx_malloc->MEMBER;
>> + CONDITIONALLY_OVERRIDE(oper_new);
>> + CONDITIONALLY_OVERRIDE(oper_new__);
>> + CONDITIONALLY_OVERRIDE(oper_delete);
>> + CONDITIONALLY_OVERRIDE(oper_delete__);
>> + CONDITIONALLY_OVERRIDE(oper_new_nt);
>> + CONDITIONALLY_OVERRIDE(oper_new___nt);
>> + CONDITIONALLY_OVERRIDE(oper_delete_nt);
>> + CONDITIONALLY_OVERRIDE(oper_delete___nt);
>> + }
>
> I don't suppose there's any way that the above could be in some sort
> of array so that the array could just be extended and new functions
> handled automatically?
I think it's important to keep the correctly-typed pointers-to-functions,
but I guess I could union them with an array of void* or something like that
so this part can be a for-loop, sure.
Righto. Respin will follow in a day or two as a submission to the -patches
list. Thanks for the fast review.
cheers,
DaveK
More information about the Cygwin-developers
mailing list