On Fri, 2010-02-19 at 10:40 +0200, Izik Eidus wrote:
> On Thu, 18 Feb 2010 21:58:51 +0100 Alexander Larsson <al...@redhat.com> wrote:
>
> red_display_send_upgrade(DisplayChannel *display_channel, UpgradeIte
> > copy->base.box = qxl_drawable->bbox;
> > copy->base.clip.type = SPICE_CLIP_TYPE_RECTS;
> > copy->base.clip.data = channel->send_data.header.size;
> > - add_buf(channel, BUF_TYPE_RAW, &item->region.num_rects,
> sizeof(uint32_t), 0, 0);
> > - add_buf(channel, BUF_TYPE_RAW, item->region.rects,
> sizeof(SpiceRect) * item->region.num_rects, 0, 0);
> > + rects = region_dup_rects(&item->region, &num_rects);
> > + num_rects32 = num_rects;
> > + add_buf(channel, BUF_TYPE_RAW, &num_rects32, sizeof(uint32_t),
> 0, 0);
> > + add_buf(channel, BUF_TYPE_RAW, rects, sizeof(SpiceRect) *
> num_rects, 0, 0);
> > + free(rects);
>
>
> So this is wrong, add_buf() will add the address of what rects is
> pointing to the
> sending vector, but then you free() it before the server send it.
>
> The way to deal with this would be freeing it at
> red_display_release_stream_clip()
> and allocating the memory at the callers of __new_stream_clip()
Ah, yeah, that sucks. Will fix.
> > + add_buf(channel, BUF_TYPE_RAW, &num_rects32,
> sizeof(uint32_t), 0, 0);
> > + add_buf(channel, BUF_TYPE_RAW, rects, num_rects *
> sizeof(SpiceRect), 0, 0);
> > + free(rects);
>
> The same problem.
Yeah.
> So about this patch just one more question:
> It look to me like there are some cases we translate rects from one
> type into another
> where we could have use one type in the begining?,
> (I did see many cases that it is a most, But some looked like we could
> avoid this?)
I agree, and it would be trivial to make SpicePoint and pixman_box32_t
identical. There are two differences atm, SpicePoint is packed, and the
coordinates are in another order. If we just changed the order and
removed the packed (which is unlikely to matter anyway in a struct with
4 int32_ts) then we could avoid all the conversions.
However, SpiceRect can't change, since its used as-is in the binary
format of the spice protocol...
Yet another reason we want to have a real demarshaller...
--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Alexander Larsson Red Hat, Inc
al...@redhat.com alexander.lars...@gmail.com
He's a notorious Amish romance novelist from the 'hood. She's a high-kicking
wisecracking barmaid in the wrong place at the wrong time. They fight crime!
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
