Re: [patch] serious problem in pango_layout_set_text()

[Owen Taylor <otaylor redhat com>]

Sven Neumann <sven gimp org> writes:

> Hi,
> 
> Owen Taylor <otaylor redhat com> writes:
> 
> > > >   g_return_if_fail (layout != NULL);
> > > >   g_return_if_fail (length == 0 || text != NULL);
> > > 
> > > you allow text == NULL && length == 0 here and this used to work before
> > > and cleared the layout. 
> > > 
> > > >   if (!g_utf8_validate (text, length, &end))
> > > >     g_warning ("Invalid UTF8 string passed to pango_layout_set_text()");
> > > 
> > > g_utf8_validate() does not like text == NULL however and will return FALSE
> > > with a warning. 
> > > 
> > > >   length = end - text;
> > > 
> > > here you use the undefined value of end which has never been touched by
> > > g_utf8_validate() ...
> > 
> > This is irrelevant. Remember, you got a *Gtk-Criticial*, all future
> > bets are off. -)
> 
> well, I've got a critical warning because I used the code as documented 
> (I take the g_return_foo code as documentation here). This should be 
> considered a Pango bug. Otherwise you could simply add

Well, what I'm saying is that all you needed to say was that you got
a critical warning, and that was a bug already. You didnt' need to
continue on and analyize what happened _after_ you got a warning to
this effect. 
> 
>     g_warning ("GTK+ comes with no warranty, all bets are off.");
> 
> to gtk_init() and declare GTK+ as bug free ;-)
> 
> > > Not sure how this would be fixed best...
> > 
> >  if (length != 0)
> >    {
> >      if (!g_utf8_validate (text, length, &end))
> >        g_warning ("Invalid UTF8 string passed to pango_layout_set_text()");
> >      length = end - text;
> >    }

I've already applied this patch.

> > Or we could make g_utf8_validate accept length == 0 && text == NULL, but that
> > doesn't seem all that useful.
> 
> I do think it is useful. It would make code dealing with buffers much easier
> if you could use g_utf8_validate on the buffer without checking length > 0.
> If I call g_utf8_validate() with length == 0, I'd expect it to tell me that
> my 0 bytes are valid UTF-8 (at least they are not invalid).

I'd accept a patch that made g_utf8_validate() allow length == 0 && text == NULL,
but I'm not sure if it makes sense to go through and do this _everywhere_ that
we take string/length combos.

Should 

 gchar *g_utf8_strup (NULL, 0); 

work and return ""? Fixing all cases would be a much bigger patch. The argument for it 
would be that:

 copy = g_malloc (len) ;
 memcpy (copy, len, orig);
 up = g_utf8_strup (copy, len);

Doesn't work if len == 0, currently. We probably should make a global decision
about this, however.

Regards,
                                        Owen