On Thu, 2016-06-09 at 05:35 -0400, Frediano Ziglio wrote:
> >
[snip]
> > > @@ -678,7 +747,7 @@ static void red_replay_string_free(SpiceReplay
> > > *replay,
> > > QXLString *qxl)
> > > red_replay_data_chunks_free(replay, qxl, sizeof(*qxl));
> > > }
> > >
> > > -static void red_replay_text_ptr(SpiceReplay *replay, QXLText *qxl,
> > > uint32_t
> > > flags)
> > > +static replay_t red_replay_text_ptr(SpiceReplay *replay, QXLText *qxl,
> > > uint32_t flags)
> > > {
> > > int temp;
> > >
> > > @@ -688,6 +757,7 @@ static void red_replay_text_ptr(SpiceReplay *replay,
> > > QXLText *qxl, uint32_t flag
> > > red_replay_brush_ptr(replay, &qxl->back_brush, flags);
> > > replay_fscanf(replay, "fore_mode %d\n", &temp); qxl->fore_mode = temp;
> > > replay_fscanf(replay, "back_mode %d\n", &temp); qxl->back_mode = temp;
> > > + return REPLAY_OK;
> > > }
> > >
> > > static void red_replay_text_free(SpiceReplay *replay, QXLText *qxl,
> > > uint32_t
> > > flags)
> > > @@ -697,9 +767,10 @@ static void red_replay_text_free(SpiceReplay *replay,
> > > QXLText *qxl, uint32_t fla
> > > red_replay_brush_free(replay, &qxl->back_brush, flags);
> > > }
> > >
> > > -static void red_replay_whiteness_ptr(SpiceReplay *replay, QXLWhiteness
> > > *qxl,
> > > uint32_t flags)
> > > +static replay_t red_replay_whiteness_ptr(SpiceReplay *replay,
> > > QXLWhiteness
> > > *qxl, uint32_t flags)
> > > {
> > > red_replay_qmask_ptr(replay, &qxl->mask, flags);
> > > + return REPLAY_OK;
> > > }
> > >
> > > static void red_replay_whiteness_free(SpiceReplay *replay, QXLWhiteness
> > > *qxl,
> > > uint32_t flags)
> > > @@ -707,9 +778,10 @@ static void red_replay_whiteness_free(SpiceReplay
> > > *replay, QXLWhiteness *qxl, ui
> > > red_replay_qmask_free(replay, &qxl->mask, flags);
> > > }
> > >
> > > -static void red_replay_blackness_ptr(SpiceReplay *replay, QXLBlackness
> > > *qxl,
> > > uint32_t flags)
> > > +static replay_t red_replay_blackness_ptr(SpiceReplay *replay,
> > > QXLBlackness
> > > *qxl, uint32_t flags)
> > > {
> > > red_replay_qmask_ptr(replay, &qxl->mask, flags);
> > > + return REPLAY_OK;
> > > }
> > >
> > > static void red_replay_blackness_free(SpiceReplay *replay, QXLBlackness
> > > *qxl,
> > > uint32_t flags)
> > > @@ -717,9 +789,10 @@ static void red_replay_blackness_free(SpiceReplay
> > > *replay, QXLBlackness *qxl, ui
> > > red_replay_qmask_free(replay, &qxl->mask, flags);
> > > }
> > >
> > > -static void red_replay_invers_ptr(SpiceReplay *replay, QXLInvers *qxl,
> > > uint32_t flags)
> > > +static replay_t red_replay_invers_ptr(SpiceReplay *replay, QXLInvers
> > > *qxl,
> > > uint32_t flags)
> > > {
> > > red_replay_qmask_ptr(replay, &qxl->mask, flags);
> > > + return REPLAY_OK;
> > > }
> > >
> > > static void red_replay_invers_free(SpiceReplay *replay, QXLInvers *qxl,
> > > uint32_t flags)
> > > @@ -727,7 +800,7 @@ static void red_replay_invers_free(SpiceReplay
> > > *replay,
> > > QXLInvers *qxl, uint32_t
> > > red_replay_qmask_free(replay, &qxl->mask, flags);
> > > }
> > >
> > > -static void red_replay_clip_ptr(SpiceReplay *replay, QXLClip *qxl)
> > > +static replay_t red_replay_clip_ptr(SpiceReplay *replay, QXLClip *qxl)
> > > {
> > > replay_fscanf(replay, "type %d\n", &qxl->type);
> > > switch (qxl->type) {
> > > @@ -735,6 +808,7 @@ static void red_replay_clip_ptr(SpiceReplay *replay,
> > > QXLClip *qxl)
> > > qxl->data = QXLPHYSICAL_FROM_PTR(red_replay_clip_rects(replay));
> > > break;
> > > }
> > > + return REPLAY_OK;
> > > }
> > > >
That reminds me. In my previous review, I was going to note that in the
functions above, you don't appear to be handling potential errors from
replay_fscanf(). But I forgot to mention it. I was going to mention it now, but
I just realized that the replay_fscanf() function is actually redefined as a
macro that returns on an error. This is why I don't like to these macros. It
makes the code difficult to understand at a glance.
> >
> >
> I'll post another version. It's quite a big change, could be
> that I miss some points. I didn't split the allocated list into a
> separate patch, I think was not clear the purpose, I put more comment
> on the commit.
>
> Frediano
ok
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel