On Thu, Nov 19, 2015 at 11:39:51AM -0500, Frediano Ziglio wrote:
> > On Wed, Nov 18, 2015 at 5:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > >
> > > ---
> > > server/cursor-channel.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> > > index aafc807..794dcf3 100644
> > > --- a/server/cursor-channel.c
> > > +++ b/server/cursor-channel.c
> > > @@ -223,7 +223,7 @@ static void put_cursor_pipe_item(CursorChannelClient
> > > *ccc, CursorPipeItem *pipe_
> > > return;
> > > }
> > >
> > > - spice_assert(!pipe_item_is_linked(&pipe_item->base));
> > > + spice_return_if_fail(!pipe_item_is_linked(&pipe_item->base));
> > >
> > > cursor_item_unref(pipe_item->cursor_item);
> > > free(pipe_item);
> > > @@ -281,7 +281,7 @@ static void red_marshall_cursor_init(RedChannelClient
> > > *rcc, SpiceMarshaller *bas
> > > SpiceMsgCursorInit msg;
> > > AddBufInfo info;
> > >
> > > - spice_assert(rcc);
> > > + spice_return_if_fail(rcc);
> > > cursor_channel = SPICE_CONTAINEROF(rcc->channel, CursorChannel,
> > > common.base);
> > >
> > > red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT, NULL);
> > > @@ -414,7 +414,7 @@ static void
> > > cursor_channel_release_item(RedChannelClient *rcc, PipeItem *item, i
> > > {
> > > CursorChannelClient *ccc = RCC_TO_CCC(rcc);
> > >
> > > - spice_assert(item);
> > > + spice_return_if_fail(item);
> > >
> > > if (item_pushed) {
> > > cursor_channel_client_release_item_after_push(ccc, item);
> > > --
> > > 2.4.3
> > >
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> > Seems okay, ACK!
> >
>
> I personally would NACK all these. Reasons:
> - there is no rationale whatsoever;
Agreed
> - with default setting the change does nothing;
Which is why using g_return_if_fail() or similar would be nice
> - actually these assert does not happen so I don't see the point
> of changing them without being able to cause the issue.
If they don't happen, we should remove them then, not keep them...
If this situation might happen, but reviewing the code shows an early
return rather than an assert will be handled properly, then
g_return_if_fail() is what we should use here rather than exposing our
users to a potential VM crash when some corner case is hit.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel