Re: [PATCH] Move custom icons when copying metadata

[Alexander Larsson <alexl redhat com>]

On Mon, 2005-09-26 at 16:49 +0200, Christian Neumair wrote:
> Am Montag, den 26.09.2005, 12:28 +0200 schrieb Alexander Larsson:
> > You're mixing g_free/xmlFree in:
> > +               property = xmlGetProp (destination_file_node, NAUTILUS_METADATA_KEY_CUSTOM_ICON);
> > +               if (property != NULL && g_str_has_prefix (property, source_file_uri)) {
> > +                       p = property;
> > +
> > +                       property = g_strconcat (destination_file_uri,
> > +                                               property + strlen (source_file_uri),
> > +                                               NULL);
> > +                       xmlSetProp (destination_file_node, NAUTILUS_METADATA_KEY_CUSTOM_ICON, property);
> > +
> > +                       xmlFree (p);
> > +               }
> > +
> > +               xmlFree (property);
> > 
> > I recommend using p for the g_strconcat result instead and then
> > g_free(p).
> 
> Maybe sb. could explain me what the influence of char signedness is in
> practice, and in what conditions I can safely "mix" xmlChar and
> (g)chars? I supposed I'm not supposed to mix them at all - hence the
> namespacing? What if the numeric value of an xmLChar is greater than the
> value a char is able to take and I pass it to g_strconcat?

The signedness is just some strange thing that libxml uses to mark some
types of strings (utf8 or something?). However, what you can't mix is
xmlFree() and g_free(). Strings allocated by libxml must be freed with
xmlFree, and strings allocated with g_malloc/g_new/etc must be freed
with g_free. The reason is that both of these malloc systems are
swappable. If you allocate with one system and free it with another
things will break if one of them isn't the standard one.

> > Its sort of strange that the metafile system special-cases a specific
> > key entry like this. It would make more sense to e.g. store the custom
> > icon with a relative filename. That would work for more cases than the
> > prefix of the uri being identical too. For instance when there are
> > symlinks etc involved.
> 
> Yup, I've had something like that on plate already. However, it didn't
> seem trivial to make a relative out of two full URIs. Also, I wanted to
> fix this in a way that does not force users to re-pick icons for their
> existing folders.

Isn't it trivial? Its basically what you patch is doing, although it is
slightly limited (must match prefix exactly). But even so it will work
much better. I.e. if you do this on a removable media the uri prefix
match will work, letting you rewrite it to a relative path. And then
when you mount it on another machine in another place the relative path
will work.

Anyway, having the metadata engine magically know about some high-level
semantics is a break of abstraction levels. Its bad design, and it will
surprise us in the future in strange ways, and likely break if we change
the metadata system (using extended attributes, a common metadata system
or whatever).

I don't think it will be a huge problem for existing icons. They will
continue working and will only be affected if you move/copy the
directories.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a scarfaced umbrella-wielding werewolf with acid for blood. She's a 
plucky impetuous opera singer on the trail of a serial killer. They fight 
crime!