Am Sonntag 21 Februar 2010 14:56:57 schrieb Bastien Nocera:
> On Fri, 2010-02-19 at 19:57 +0100, Hendrik Sattler wrote:
> > Hi Bastien,
> >
> > I have some comments for your patches.
>
> <snip>
>
> > > + while ((usbfd = usbfds[i++]) != NULL) {
> > > + if (usbfd->events & POLLOUT)
> >
> > This is not correct. The file descriptor is used in
> > obex_transport_handle_input() in context of the select() funcion. But
> > that only waits for incoming data, not for outgoing. So you are choosing
> > the wrong file descriptor, here, use the one for POLLIN instead.
>
> You're right. It was a bug in the code I used on top of it that created
> the error.
>
> > > + return usbfd->fd;
> > > + }
> > > +
> > > + return INVALID_SOCKET;
> > > +}
> > > +
> > > +/*
> > > * Function usbobex_connect_request (self)
> > > *
> > > * Open the USB connection
> > > @@ -379,6 +409,7 @@ int usbobex_connect_request(obex_t *self)
> > > }
> > >
> > > self->trans.mtu = OBEX_MAXIMUM_MTU;
> > > + self->fd = usbobex_get_fd();
> > > DEBUG(2, "transport mtu=%d\n", self->trans.mtu);
> > > return 1;
> >
> > Additionally to setting self->fd here, you should also modify
> > obex_transport_handle_input() to no choose the special USB path when
> > self->fd is valid.
>
> Except that it doesn't work because it doesn't actually select on
> self->fd, but on self->fd+1.
>
> ret = select((int)highestfd+1, &fdset, NULL, NULL, &time);
>
> I'm pretty sure that's not correct for our case.
The select call is correct. The change in obex_transport_handle_input() can be
an additional patch.
> > > From: Bastien Nocera <had...@hadess.net>
> > > Date: Fri, 19 Feb 2010 13:40:27 +0000
> > > Subject: [PATCH] Fix invalid memory access
> > >
> > > The hdr pointer will not be valid any more if the transport
> > > read() does a realloc, so cache the opcode that we'll use
> > > later in the code.
> >
> > Not the correct approach, in my eyes. Instead, update msg (also used
> > after this point) and hdr variables.
> > Good finding :)
>
> msg wouldn't change. It's a pointer to a struct which contains a pointer
> to the actual data. Only ->data would change, not the msg pointer
> itself. Fixed in the patch.
You are right.
Looks good now. I added them to my repository at gitorious.org/openobex. Maybe
Marcel can pull them from there.
HS
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Openobex-users mailing list
Openobex-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/openobex-users
