After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 638838 - gdesktopappinfo: Don't crash if we don't have a desktop filename
gdesktopappinfo: Don't crash if we don't have a desktop filename
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 638937 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-01-06 16:48 UTC by Colin Walters
Modified: 2011-02-03 13:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdesktopappinfo: Don't crash if we don't have a desktop filename (2.53 KB, patch)
2011-01-06 16:48 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2011-01-06 16:48:56 UTC
If code creates a GDesktopAppInfo via g_desktop_app_info_new_from_keyfile(),
we'd try to send a NULL pointer down into GVariant.

Since in this case we don't have a filename, just send the empty
string.  In the future we should either:

1) Change panel to use g_desktop_app_info_new_from_filename(), and
   take the hit of parsing the file twice.
2) Add a g_key_file_get_origin_filename()
3) Add g_desktop_app_info_new_from_keyfile_and_name()
Comment 1 Colin Walters 2011-01-06 16:48:58 UTC
Created attachment 177668 [details] [review]
gdesktopappinfo: Don't crash if we don't have a desktop filename
Comment 2 Colin Walters 2011-01-06 18:10:42 UTC
<seb128> walters, I can confirm that your patch fixes the gnome-panel crash there
Comment 3 Dan Winship 2011-01-06 18:21:30 UTC
Comment on attachment 177668 [details] [review]
gdesktopappinfo: Don't crash if we don't have a desktop filename

good, just nitpicks...

>-		       const char       *desktop_file, /* filename */
>+		       GDesktopAppInfo  *info, /* filename */

the comment is wrong now

>+  const guint8 *desktop_file_id;

it's silly to have this be a guint8* since that requires a cast to or from char* every time you use it.

>+  if (info->filename)
>+    desktop_file_id = (guint8*) info->filename;
>+  else if (info->desktop_id != NULL)
>+    desktop_file_id = (guint8*) info->desktop_id;

one branch has "!= NULL", the other doesn't.
Comment 4 Colin Walters 2011-01-06 18:31:10 UTC
Attachment 177668 [details] pushed as e738a8d - gdesktopappinfo: Don't crash if we don't have a desktop filename
Comment 5 Vincent Untz 2011-01-08 12:00:49 UTC
*** Bug 638937 has been marked as a duplicate of this bug. ***
Comment 6 Mikkel Kamstrup Erlandsen 2011-02-03 13:36:38 UTC
Just for reference. Solution 3) from the bug description has long had a ready-to-ship patch on https://bugzilla.gnome.org/show_bug.cgi?id=638838. And I think that is needed to solve the deeper issue here - namely that appinfos will often be created without and id or filename.