Re: [gtk-osx-devel] [PATCH] gtk-mac-integration reference counting

[John Ralls <jralls ceridwen us>]

On Jan 29, 2012, at 11:46 AM, Richard Procter wrote:

> 
> On 29/01/2012, at 6:01 PM, John Ralls wrote:
>> On Jan 28, 2012, at 2:09 PM, Richard Procter wrote:
>>> I ran the latest clang static analyzer [1] over gtk-mac-integration - here's a patch for a couple of ref-counting issues it found. Nothing that looks crucial but nice to have all the same. I've compiled it but have not tested it and as I rarely use objective-c it's worth casting your eye over it. 
>> [snip]
>> What SDK are you analyzing against? Apple has some new memory-management features in 10.7, and they did some major cleanup for 10.6. All well and good, but gtk-mac-integration has to work correctly with 10.4.
> 
> Is that the ARC stuff? I'm building on 10.6 and haven't used that in the patch. Every change is to do with adding 'autorelease' to any object created with [alloc] or [new] without a corresponding explicit [release], or to do with removing [release] calls for objects we do not own. 
> 
>> Functions need to keep the gtk_mac name (in fact, 1.1 will rename them to gtkmac_) for compatibility with 
>> gobject-introspection.  
> 
> OK, good to know. Does that mean that gtk_mac_image_from_pixbuf() is exposed in the user's API and shouldn't be renamed? I have split up the patch to separate this rename, and changing it to gtk_mac_create_cgimage_from_pixbuf(). The name is significant because it violates the core foundation 'create rule' as it stands, returning a CGImage owned by the caller, which is what clang complained about. 
> 
>> checker-260/bin/clang? You're not using Apple's?
> 
> No - it's more recent than the one in the XCode 4.2 release - output below. 
> 
> best, 
> Richard. 
> 
> <mac_integration_ref_counting.patch>
> 
> <mac_integration_rename_gtk_mac_image.patch>
> 
> 
> 
> cocoa_menu_item.c:165:23: warning: Potential leak of an object allocated on line 165 and stored into 'cocoa_submenu'
>      cocoa_submenu = [ [ NSMenu alloc ] initWithTitle:@""];
>                      ^
> cocoa_menu_item.c:163:4: warning: Potential leak of an object allocated on line 163
>                        [[ NSString alloc] initWithUTF8String:label_text]];
>                        ^
> cocoa_menu_item.c:162:23: warning: Potential leak of an object allocated on line 162 and stored into 'cocoa_submenu'
>      cocoa_submenu = [ [ NSMenu alloc ] initWithTitle:
>                      ^
> cocoa_menu_item.c:192:26: warning: Potential leak of an object allocated on line 192
>    [cocoa_item setTitle:[ [ NSString alloc] initWithCString:label_text encoding:NSUTF8StringEncoding]];
>                         ^
> cocoa_menu_item.c:487:22: warning: Potential leak of an object allocated on line 487
>                     initWithTitle:[ [ NSString alloc]
>                                   ^
> cocoa_menu_item.c:492:20: warning: Potential leak of an object allocated on line 492 and stored into 'cocoa_item'
>      cocoa_item = [ [ GNSMenuItem alloc] initWithTitle:@""
>                   ^
> cocoa_menu_item.c:486:20: warning: Potential leak of an object allocated on line 486 and stored into 'cocoa_item'
>      cocoa_item = [ [ GNSMenuItem alloc]
>                   ^
> 7 warnings generated.
> gtk-mac-image-utils.c:51:11: warning: Potential leak of an object allocated on line 51 and stored into 'image'
>  image = CGImageCreate (pixbuf_width, pixbuf_height, 8,
>          ^
> 1 warning generated.
> gtkosxapplication_quartz.c:180:22: warning: Potential leak of an object allocated on line 180 and stored into 'app_menu'
>  NSMenu *app_menu = [[NSMenu alloc] initWithTitle: @"Apple Menu"];
>                     ^
> gtkosxapplication_quartz.c:182:26: warning: Potential leak of an object allocated on line 182 and stored into 'menuServices'
>  NSMenu *menuServices = [[NSMenu alloc] initWithTitle:  NSLocalizedStringFromTable(@"Services",  @"GtkOSXApplication", @"Services Menu title")];
>                         ^
> gtkosxapplication_quartz.c:237:25: warning: Potential leak of an object allocated on line 237 and stored into 'window_menu'
>  NSMenu *window_menu = [[NSMenu alloc] initWithTitle: NSLocalizedStringFromTable(@"Window",  @"GtkOSXApplication", @"Window Menu title")];
>                        ^
> gtkosxapplication_quartz.c:474:24: warning: Potential leak of an object allocated on line 474
>  [ NSApp setDelegate: [GtkApplicationDelegate new]];
>                       ^
> gtkosxapplication_quartz.c:655:21: warning: Potential leak of an object allocated on line 655 and stored into 'cocoa_menubar'
>    cocoa_menubar = [[GNSMenuBar alloc] initWithGtkMenuBar: 
>                    ^
> gtkosxapplication_quartz.c:761:17: warning: Value stored to 'menubar' during its initialization is never read
>    GtkMenuBar *menubar = [cocoa_menubar menuBar];
>                ^         ~~~~~~~~~~~~~~~~~~~~~~~
> gtkosxapplication_quartz.c:799:33: warning: Potential leak of an object allocated on line 799
>    [cocoa_menubar setHelpMenu: [[GNSMenuItem alloc] initWithTitle: @"Help"
>                                ^
> gtkosxapplication_quartz.c:875:5: warning: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
>    [ns_name release];
>    ^~~~~~~~
> gtkosxapplication_quartz.c:881:3: warning: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
>  [ns_name release];
>  ^~~~~~~~
> gtkosxapplication_quartz.c:920:3: warning: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
>  CGImageRelease (image);
>  ^               ~~~~~
> gtkosxapplication_quartz.c:1109:5: warning: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
>    [id release];
>    ^~~
> gtkosxapplication_quartz.c:1113:3: warning: Incorrect decrement of the reference count of an object that is not owned at this point by the caller
>  [id release];
>  ^~~ 
> 12 warnings generated.
> test-integration.c:728:3: warning: Value stored to 'window1' is never read
>  window1 = create_window("Test Integration Window 1");
>  ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 warning generated.
> 

Please remember to use "reply all".

ARC is the feature added in 10.7. There were also memory management changes between 10.5 and 10.6. It would be wise to build against the 10.4SDK and compare the analysis from that with the results from building against SDK 10.6. It might be necessary to #ifdef some of the changes by version. I don't know, the only way to be sure is to do the check.

For the autoreleased objects, did you make sure that the receivers retain them in all cases?

Yes, gtk_mac_create_image_from_pixbuf is exposed in gtk_mac_image_utils.h. It's not part of GtkOsxApplication, but it's still useful for things like setting the dock tile icon if you're going to have it change with state. Clang is reporting a potential memory leak, not offering advice about naming conventions. Hmm, that's going to cause memory trouble with introspection, too. I need to make a note about that.

Regards,
John Ralls