On Fri, 12 Nov 2010 15:16:33 +0100
Markus Armbruster <arm...@redhat.com> wrote:
> Luiz Capitulino <lcapitul...@redhat.com> writes:
>
> > On Fri, 12 Nov 2010 11:21:57 +0100
> > Markus Armbruster <arm...@redhat.com> wrote:
> >
> >> Luiz Capitulino <lcapitul...@redhat.com> writes:
> [...]
> >> > +QString *qemu_chr_mem_to_qs(CharDriverState *chr)
> >> > +{
> >> > + MemoryDriver *d = chr->opaque;
> >> > +
> >> > + if (d->outbuf_size == 0) {
> >> > + return qstring_new();
> >> > + }
> >>
> >> Why is this necessary? Is qstring_from_substr() broken for empty
> >> substrings? If it is, it ought to be fixed!
> >
> > qstring_from_substr() takes a character range; outbuf_size stores a size,
> > not a string length. So we do:
> >
> >> > + return qstring_from_substr((char *) d->outbuf, 0, d->outbuf_size -
> >> > 1);
> >
> > If outbuf_size is 0, we'll be passing a negative value down.
>
> What's wrong with that?
Although it's going to work with the current QString implementation, I don't
think it's it's a good idea to rely on a negative index.
Maybe, we could have:
return qstring_from_substr((char *) d->outbuf, 0,
d->outbuf_size > 0 ? d->outbuf_size - 1 : 0);
A bit harder to read, but makes the function smaller.
