On Nov 11, 2010, at 8:42 AM, Christiaan Hofman wrote: However, you have neglected to CFRelease (or release, or autorelease) the value stored in axValue by the AXUIElementCopyAttributeValue() function call in the body of your method The Cocoa convention is that a method like your -valueForAttributeCopy: always returns an object autoreleased. So, you might want to add '[axValue autorelease]' or '[value autorelease]' before the return, or change the last line to 'return [value autorelease]', and see what happens. (If the caller of your method does releases its return value when done with it, remove that release because it would be automatically released by the autorelease added in your method).
Actually, no. The docs (Memory Management Programming Guide) say that the returned value in this case should be retained:
"You “create” an object using a method whose name begins with “alloc” or “new” or contains “copy” "
Note the word "contains", which only applies to "copy".
(I personally think this rule is wrong, or at least formulated wrongly and inconsistently in the docs. For instance compare a method initWithObjects:copyItems: which clearly *contains* the word "copy" but whose return value isn't explicitly retained. But Apple does not agree with me, and they refuse to be unambiguous in the docs, even though it's pretty straightforward to be clear.)
Apart from this, I would advice to just avoid any confusion, and rename the method: either name it -valueForAttribute: and autorelease the value, or name it -copyValueForAttribute:.
I wasn't clear in my response. I was addressing the Core Foundation "copy" rule, which is similar to but perhaps theoretically separate from the Cocoa naming convention you cite.
When I cited the rule that values returned by Core Foundation functions containing the word "copy" must be CFReleased when no longer needed, I was referring to the OP's call to the Accessibility API function AXUIElementCopyAttributeValue() in the body of his method. That function contains the word "copy," which tells us that its indirectly returned value has been CFRetained. He is therefore responsible for CFReleasing the object that is returned indirectly in the function's axValue argument, but he does not do that in the body of his method.
It is entirely possible, of course, that the OP did in fact intend to return it from his Cocoa method CFRetained, and that he therefore deliberately included the word "copy" in his method name in order to comply with the Cocoa naming convention. That would be perfectly correct and appropriate, as you point out. But in that case, it would be his responsibility to release the return value from his method when he is done with it. Perhaps he in fact did so (he didn't show us his calling code), but I was guessing that the memory leak he was experiencing came from a failure to do so.
I agree with your suggestion that he should remove the word "copy" from the name of his method if he decides to autorelease the return value. However, I fear that your suggestion that the value returned by his method as it is currently named should be retained because it does contain the word "copy" may confuse him. You have correctly described the naming convention, but in this case the return object of his method is already retained (or, I should say, CFRetained) because it was obtained from an Accessibility API function containing the word "copy." If this was his intent and he properly released the value later, then he doesn't have to change anything and his memory leak is caused by something else entirely.
Turning to your comment on a hypothetical -initWithObjects:copyItems: method in Cocoa, I wonder if you aren't taking the Cocoa naming convention a little too literally. It is true that the method signature contains the word "copy," but it seems clear to me in this case that "copy" is associated with "items" and does not refer to the returned object. The Cocoa methods that include "copyItems" in their signatures, such as -initWithArray:copyItems:, are documented as copying (not retaining) the items in the array. This might be distinguished from the Application Services function PMWorkflowCopyItems(), which is documented to require that the caller release the result. |