Re: Support for major_mime_type/*



    
Hi Gergo,
        
        Sorry for ignoring your patch, I must have been away somewhere    
when you posted it, and somehow missed it in catching up.

On Sat, 21 Apr 2001, ERDI Gergo wrote:
> Before beginning to investigate what it would take to move the
> monikers to gnome-vfs, I decided to make this very small hack to
> Bonobo Classic (:)) to enable components to support every mime type
> that has a given major mime type (i.e. a component that supports
> text/*). Is it OK to check in?
 
        Ok, the overall idea looks great thanks for the work - I have ( as
always ) several concerns :-)
 
> * Yes, get_major_mime_type () is ugly, it shouldn't be c&p'd over the
> two extenders, in fact, it shouldn't be in there at all. I know all of
> that and I will of course use gnome_vfs_get_supertype_from_mime_type
> in the gnomevfs version.
 
        Can we put this inside bonobo-stream.c as
bonobo_internal_get_major_mime_type, document it as only being used in the
extenders, not add it to the header and put extern prototypes in the
moniker extenders. I don't want to go round adding new functions that
people use, but I want to share this code - just ugly in a different way
:-)
 
        Now lets look at the function:
 
+/* The result needs to be g_free'd */
+static gchar* get_major_mime_type (const char* mime_type)

	Can prototypes be like this:

static gchar *
get_major_mime_type (const char *mime_type)

	This is so you can do grep '^my_function_name' *.c and find the
impl trivialy.

+{
+       char *major_end;
+       char *major;
+       int   major_length;
+
+       major_end = strchr (mime_type, '/');
 
        add:
 
        if (!major_end)
                return g_strdup (mime_type);
 
        or there'll be trouble :-)
 
        The following section has redundant and somewhat strange comments
:-) I tend to try to pitch my source code at somewhere above a moron, and
below a very experienced hacker -
 
+       major = g_new (gchar, major_length + 1); /* +1 for trailing \0 */
+       strncpy (major, mime_type, major_length);
+       major[major_length] = '\0'; /* Uglee */

        Either way, I would prefer you to use:

        return g_strndup (mime_type, major_length);

        Apart from that, it looks fine - can you make the changes, and
send me a patch as you commit - also if you could checkout tmpbonobo and  
make the mods to the moniker code there [blind] that'd be great - just so
I remember it for G2.

	Thanks,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot





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