A patch for uniform mime handling



Hello,
    I am emailing this list to propose a patch to gnome_vfs_get_mime_type to return
GNOME_VFS_MIME_TYPE_UNKNOWN instead of NULL as it does now.  The patch on bug 121553
(http://bugzilla.gnome.org/show_bug.cgi?id=121553) fixes this issue and also handles it
the way gnome_vfs_get_mime_type_common, gnome_vfs_get_file_mime_type,
gnome_vfs_get_mime_type_for_data, and gnome_vfs_get_mime_type_from_file_data (by using
_gnome_vfs_get_mime_type_internal instead of returning NULL).  Not only does it make the
function more uniform to the other 4 functions, but it also fixes a crash on Solaris when
the function returns NULL (because you can't do printf("%s\n", NULL);).
    You may be saying to yourself, "This breaks API", but I would say to you that all
apps that use this function have to handle this anyway, and most set the mime-type to
GNOME_VFS_MIME_TYPE_UNKNOWN themselves. For example:

At line 97 of src/egg-recent-item.c in totem cvs HEAD:
	item->mime_type = gnome_vfs_get_mime_type (item->uri);

	if (!item->mime_type)
		item->mime_type = g_strdup (GNOME_VFS_MIME_TYPE_UNKNOWN);

As you can see, this code just works around the NULL return and wouldn't break with the
new patch. EOG, File-Roller, gedit, gpdf, nautilus

Another example is at line 1027 of lib/rb-playlist.c in rhythmbox cvs HEAD:
	mimetype = gnome_vfs_get_mime_type (url);

	if (mimetype == NULL)
		return rb_playlist_add_url_from_data (playlist, url);

rb_playlist_add_url_from_data calls a hack to open up the file and read the mime-type
using gnome_vfs_get_mime_type_for_data (which looks up mime-types the same way the patch
will make gnome_vfs_get_mime_type look them up).  Totem uses a similar hack in
src/totem-playlist.c at line 1935.

Other applications do an if(mime_type){} check and then compare the mime-type to a
string, so returning GNOME_VFS_MIME_TYPE_UNKNOWN wouldn't affect them.  To me, the extra
if check is wasted processor cycles (but I could be wrong).  All-in-all, after a grep
through all of my cvs trees, there are only roughly 17 apps that use this function.  Here
is a rundown of the apps:

Apps that use the egg code (this change wouldn't affect them):
eog, file-roller, gedit, gnome-panel, gpdf, rhythmbox, totem

Apps that call it, but their code wouldn't break:
rhythmbox, totem, abiword (doesn't even check for NULL), yelp, nautilus,
gnome-control-center (libsounds), gnome-utils (gsearchtool), gedit, epiphany, bug-buddy,
anjuta

This list is rough (from a grep -rnI gnome_vfs_get_mime_type of my cvs sources), but I
have personally looked at the code for all of these apps and from what I see, they would
not be broken with this change.  Again, this patch will bring uniformity to the mime_type
functions, prevent a crash on Solaris, and also save programs from using costly
workarounds (especially in totem and rhythmbox's cases).  I appreciate any feedback on
this, and look forward to discussing it with you all.

-Bryan Forbes

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]