This would be a nice addition to handle GSList's as returned from some functions (e.g. gtk_file_chooser_get_uris).
Created attachment 126759 [details] Patch
Comment on attachment 126759 [details] Patch Looks good
Comment on attachment 126759 [details] Patch Ok! Where will that new smart pointer will be used? Can you "migrate" some existing code as part of this patch?
Created attachment 126763 [details] Patch (In reply to comment #3) > (From update of attachment 126759 [details]) > Ok! Where will that new smart pointer will be used? Can you "migrate" some existing code as part of this patch? It will be used in the code I'm now writing to implement the WebUIClient's runOpenPanel() feature in WebKit2GTK+, when handling the list of fileURIs as returned by gtk_file_chooser_get_uris(), which happens to be an GSList. The same kind of usage is already in WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp, so I guess it makes sense to migrate that code along with this patch, thus I'm attaching a new one incorporating that suggestion.
Comment on attachment 126763 [details] Patch Awesome! Thanks!
Committed r107566: <http://trac.webkit.org/changeset/107566>
Comment on attachment 126759 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126759&action=review > Source/JavaScriptCore/wtf/gobject/GOwnPtr.cpp:42 > + g_slist_free(ptr); You should check if the value is null before freeing it. While g_slist_free may properly handle null values, it isn't documented, so a check is safer.
(In reply to comment #7) > (From update of attachment 126759 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126759&action=review > > > Source/JavaScriptCore/wtf/gobject/GOwnPtr.cpp:42 > > + g_slist_free(ptr); > > You should check if the value is null before freeing it. While g_slist_free may properly handle null values, it isn't documented, so a check is safer. No, you shoulnd't, NULL is a valid empty GSList.
(In reply to comment #8) > > You should check if the value is null before freeing it. While g_slist_free may properly handle null values, it isn't documented, so a check is safer. > > No, you shoulnd't, NULL is a valid empty GSList. The documentation for g_slist_free seems lacking, then. :/
"There is no function to create a GList. NULL is considered to be the empty list so you simply set a GList* to NULL."
(In reply to comment #10) > "There is no function to create a GList. NULL is considered to be the empty list so you simply set a GList* to NULL." The g_slist_free makes no reference to this fact nor does it explicitly say that it handles empty GLists.
Because all GList and GSList API accept NULL due to the fact that NULL is a valid empty list as documented in the description section of the documentation.
(In reply to comment #12) > Because all GList and GSList API accept NULL due to the fact that NULL is a valid empty list as documented in the description section of the documentation. This is just an assumption one would make from reading the documentation though. All that's explicitly stated is that NULL is a valid GList/GSList. Indeed here is someone who made a mistake because of this: http://code.google.com/codesearch#-q96OtJF3X4/trunk/xdelta1/libedsio/edsio.c&q=g_list_free&type=cs&l=1501 It's rare that documentation is too-explicit. I don't think situations like these are the users' fault. This debate probably doesn't belong on this bug though...
(In reply to comment #13) > (In reply to comment #12) > > Because all GList and GSList API accept NULL due to the fact that NULL is a valid empty list as documented in the description section of the documentation. > > This is just an assumption one would make from reading the documentation though. All that's explicitly stated is that NULL is a valid GList/GSList. g_list_free() receives a GList, NULL is a valid empty GList as documented, so GList can receive NULL.