GNOME Bugzilla – Bug 320242
g_return_* macros in static functions
Last modified: 2008-10-12 19:30:07 UTC
Please describe the problem: All details here : http://mail.gnome.org/archives/performance-list/2005-October/msg00005.html Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information: Patch to follow...
Created attachment 54084 [details] [review] Replace g_return_* macros by g_assert or if () return If g_assert can be deleted, it's a better solution.
Comment on attachment 54084 [details] [review] Replace g_return_* macros by g_assert or if () return Doesn't work
Created attachment 54085 [details] [review] Replace g_return_* macros by g_assert or if () return
Am I missing something or are those if tests reversed?
This looks indeed wrong to me: model_hash = g_hash_table_lookup (ds->vendors, vendor); - g_return_val_if_fail (model_hash != NULL, NULL); + if (model_hash != NULL) { + g_free (vendor); + g_free (model); + return NULL; + }
Really silly, I will update this patch from gnome-cups-manager HEAD and without smoke crack :)
Code extract: model_hash = g_hash_table_lookup (ds->vendors, vendor); g_return_val_if_fail (model_hash != NULL, NULL); drivers = g_hash_table_lookup (model_hash, ppd_model); g_free (vendor); g_free (model); If g_return_val_if_fail is TRUE, we have a nice memory leak but only for the developper because the goal is to compile with G_DISABLE_ASSERT in final releases. I think it's better to crash application in devel phase with: model_hash = g_hash_table_lookup (ds->vendors, vendor); g_assert (model_hash != NULL); drivers = g_hash_table_lookup (model_hash, ppd_model); g_free (vendor); g_free (model); Like says John Rice (see link to mailing list) graceful failure instead of aborting, masks bugs that we should be catching during debug. If you are agree with me, patch will be simple, aren't you ?
What do you think about my previous comment? I need an answer to rewrite my previous patch.
Ping
Sure, let's try it out at least. Not sure how many apps still use gnome-print though?
Moving all gnome-cups-manager bugs to new product. Filter on Kjartan's spring cleaning.
Created attachment 117559 [details] [review] Remove the g_return_* macros from static private functions Use g_assert to check 'model_hash'
Can I commit the last patch to close this old bug report?
Created attachment 120458 [details] [review] Remove a useless test gtk_tree_model_get_iter_first (model, &iter) already checks the model.