Added Tino on CC

Created attachment 47816 [details]
a little patch for sal. No problem, but needs confirmation

Created attachment 47817 [details]
another patch for dtrans

The real issue is imho in dtrans ( leaks confirmed) , but this is a complicated issue: objects including 
CFStrings or CFAreaRef or .. are instanciated, and I found nothing obvious explaining how everything is 
freed. 

e.g; every time we copy paste, new leaks appear, confirming the root cause is in dtrans aqua.

ericb->tino

In dtrans/source/aquaDataFlavorMapping.cxx, getLegacyClipboardId()  is not used at all. 
Can you confirm ?

tra->ericb: I reviewed my code and am not sure if 'leaks' is not reporting false positives. In the first 
case OSXTransferable.cxx I'm using the function 'CFArrayGetValueAtIndex'. According to the 
documentation "...Return Value
The value at the idx index in theArray. If the return value is a Core Foundation Object, ownership 
follows the Get Rule...". The "Get Rule" says "If you receive an object from any Core Foundation function 
other than a creation or copy function—such as a Get function—you do not own it and cannot be 
certain of the object’s life span. If you want to ensure that such an object is not disposed of while you 
are using it, you must claim ownership (with the CFRetain function). You are then responsible for 
relinquishing ownership when you have finished with it." 
I would interpret this as "I don't have to call CFRelease in my concrete case. Similar arguments count for 
the second case where I return a CFStringRef to a static immutable CFString object in 
'DataFlavorMapping::openOfficeToSystemDataFlavor' so I don't see a reason for CFRelease here as well 
even in the exception case.
But anyway leave the issue open I will re-investigate and try what happens when I use CFReleases 
nevertheless at the places you are suggesting when I'm done with the D&D implementation. 

tra->ericb: Regarding 'getLegacyClipboardId' it seems as if this function is really not used atm. I think 
planned to use it but didn't in the end. But the code needs some restructuring yet making the function 
necessary again so I would first remove it when the D&D implementation is done.
If you would create two separate issue for the leaks one for sal and another one for dtrans I would add the 
dtrans issue to cws macosxdnd.

tra->ericb: Regarding the sal patch: Although the docu doesn't say so I could imagine that the use of 
'CFStringConvertEncodingToIANACharSetName' requires a CFRelease for the use of 
'CFArrayGetValueAtIndex' the same comments as above apply in my opinion.

ericb->tra

I'll answer you for the first part asap (currently searching for a breaker in m226 )

About sal patch, if you read just two lines above, we have (yes, an extra empty line interfered):

	CFStringRef	sCFEncodingName;   

	sCFEncodingName = CFStringConvertEncodingToIANACharSetName( cfEncoding );
	CFStringGetCString( sCFEncodingName, buffer, bufferLen, cfEncoding );
      if ( sCFEncodingName )   
          CFRelease( sCFEncodingName);

What I understand, is you are the owner of the CFStringRef, and you must release it.

For the second case:

	CFStringRef lang = (CFStringRef)CFArrayGetValueAtIndex(subs, 0);
	CFStringGetCString(lang, locale, bufferLen, kCFStringEncodingASCII);
      if ( lang )
          CFRelease( lang);

I understand the same, and imho, you are the owner of the CFString too.

( waiting, I'll read again Get and SET rules anyway :-)

ericb->tra

In sal, I have removed the if it the two hunks, and so far, no crash, copy paste and everything works.

I think I'd have at least one if ever the CFRelease() were wrong. To complete, I'll trace a bit, insetring a 
breakpoint just before the release

ericb->tra

I discussed both patches with pl

In sal, release lang is wrong, because the array is autoreleased when the
function gets out of scope.
For the second, pl agreed CFRelease( sCFEncodingName) is correct, and the change
is really needed.

In dtrans, things are not clear, because entries are released, but not the Array
itself, and more investigations are needed.

To summarize, on change can be applied in sal, and we'll see later what exactly
needs to be done.

Setting keyword

@tino : can you please confirm ?

Since macosxdnd, the dtrans patch seems to be obsolete

I'll ask pl if I can add the change in sal only. aquavcl05 could be ok

Tjhe change about CFRelease( sCFEncodingName), discussed on IRC (long time ago)  with Philipp has been 
commited.

Issue fixed

target

verified in CWS aquavcl05

This seems to be a developer issue, I cant edit it. please help, and confirm or
close it - thanks

Closing resolved issue.