Hi,
On Sat, May 14, 2016 at 10:25:46PM +0200, Fabiano Fidêncio wrote:
> Toso,
>
> On Sat, May 14, 2016 at 10:09 PM, Victor Toso <lists@xxxxxxxxxxxxxx> wrote:
> > Hi,
> >
> > On Sat, May 14, 2016 at 05:56:45PM +0200, Fabiano Fidêncio wrote:
> >> On May 14, 2016 17:29, "Victor Toso" <victortoso@xxxxxxxxxx> wrote:
> >> >
> >> > We are checking self->priv->error but accessing the argument GError *
> >> > which is NULL and leads to a segfault.
> >> >
> >> > Program received signal SIGSEGV, Segmentation fault.
> >> > spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
> >> error=0x0) at channel-main.c:2963
> >> > 2963 VDAgentFileXferStatusMessage msg = {
> >> > (gdb) bt
> >> > #0 spice_file_transfer_task_completed (self=self@entry=0x7fffd0006f00,
> >> error=0x0) at channel-main.c:2963
> >> > #1 in file_xfer_data_flushed_cb (source_object=0x7cc1d0, res=0x953390,
> >> user_data=user_data@entry=0x7fffd0006f00) at channel-main.c:1857
> >> > #2 in g_task_return_now (task=0x953390) at gtask.c:1108
> >> > #3 in g_task_return (task=0x953390, type=<optimized out>) at
> >> gtask.c:1166
> >> > #4 in flush_foreach_remove (key=<optimized out>, value=<optimized out>,
> >> user_data=<optimized out>) at channel-main.c:928
> >> > #5 in g_hash_table_foreach_remove_or_steal (hash_table=0x70cea0,
> >> func=func@entry=0x7ffff5616f10 <flush_foreach_remove>,
> >> user_data=user_data@entry=0x0, notify=notify@entry=1) at ghash.c:1492
> >> > #6 in g_hash_table_foreach_remove (hash_table=<optimized out>,
> >> func=func@entry=0x7ffff5616f10 <flush_foreach_remove>,
> >> user_data=user_data@entry=0x0) at ghash.c:1538
> >> > #7 in file_xfer_flushed (success=0, channel=0x7cc1d0) at
> >> channel-main.c:936
> >> > #8 spice_main_channel_reset_agent (channel=0x7cc1d0) at
> >> channel-main.c:466
> >> > #9 set_agent_connected (channel=0x7cc1d0, connected=connected@entry=0)
> >> at channel-main.c:1572
> >> > #10 in spice_main_channel_reset (channel=0x7cc1d0, migrating=0) at
> >> channel-main.c:485
> >> > #11 in spice_channel_coroutine (data=0x7cc1d0) at spice-channel.c:2564
> >> > #12 in coroutine_trampoline (cc=0x7cb860) at coroutine_ucontext.c:63
> >> > #13 in continuation_trampoline (i0=<optimized out>, i1=<optimized out>)
> >> at continuation.c:55
> >> > #14 in ?? () from /lib64/libc.so.6
> >> > #15 in ?? ()
> >> > #16 in ?? ()
> >> > Backtrace stopped: Cannot access memory at address
> >> > ---
> >> > src/channel-main.c | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/src/channel-main.c b/src/channel-main.c
> >> > index 2905d7b..692cfe7 100644
> >> > --- a/src/channel-main.c
> >> > +++ b/src/channel-main.c
> >> > @@ -2964,7 +2964,7 @@ static void
> >> spice_file_transfer_task_completed(SpiceFileTransferTask *self,
> >> > if (self->priv->error) {
> >> > VDAgentFileXferStatusMessage msg = {
> >> > .id = self->priv->id,
> >> > - .result = error->code == G_IO_ERROR_CANCELLED ?
> >> > + .result = self->priv->error->code == G_IO_ERROR_CANCELLED ?
> >> > VD_AGENT_FILE_XFER_STATUS_CANCELLED :
> >> VD_AGENT_FILE_XFER_STATUS_ERROR,
> >> > };
> >> > agent_msg_queue_many(self->priv->channel,
> >> VD_AGENT_FILE_XFER_STATUS,
> >> > --
> >> > 2.5.5
> >> >
> >>
> >> Victor, as we talked on IRC your patch solves the problem, fust to comments:
> >>
> >> First one, shouldn't we emmit self->priv->error instead of error in the end
> >> of this function?
> >>
> >> Second one, from a quick look in the code, I have the impression that we
> >> can get rid of the private Error variable and just use the error that we
> >> are propagating between the callbacks. A deeper read in the code would be
> >> needed though in order to affirm that.
> >>
> >> Best Regards,
> >
> > I would rather that we fix the segfault in one patch and improve the
> > logic in a different patch/series. I plan to look into this code
> > carefully during this next week but I hope your suggestions are not a
> > blocker.
>
> The suggestion about emitting the signal, if needed, is a blocker.
You are right, we should emit the self->priv->error as we
g_clear_error(error).
> The other one may be done while you re-work this code path.
Great! I'll be sending the fix for this shortly.
toso
>
> >
> > toso
>
>
> Best Regards,
> --
> Fabiano Fidêncio
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel