Possible poll-related problem in dbus-0.92

[Daniel P. Berrange]

On Thu, Sep 14, 2006 at 12:08:15PM +0100, Julian Seward wrote:
> 
> Upon investigation, it seems to me that dbus is depending on a boundary
> case of poll() which is poorly specified by POSIX.  I am not saying that
> dbus necessarily has a bug, nor that valgrind is being too strict; but
> it does appear that dbus and valgrind disagree, and from reading the POSIX
> specification for poll() it's not clear what should happen.  See 
> http://www.opengroup.org/onlinepubs/007908799/xsh/poll.html for the POSIX
> spec.
> 
> The problem case is when poll() (hence _dbus_poll) returns zero.  At 
> dbus-transport-unix.c:1032 we have 
> 
>       if (poll_res >= 0)
>         {
>            /* poke around in the .revents field of the pollfd structure */
>         }

That condition should be change to 'poll_res > 0', since .revents is undefined
if poll_res == 0 according to the man pae

> 
> As currently set up, valgrind's handler for poll() does not mark the .revent
> fields as defined in the case where poll returned zero (meaning no file
> descriptors were ready).  Is that a bug?  I don't know.  It could be argued
> that dbus should not look at the .revents field in this case.

The (POSIX) man page for poll() says

 'On success, a positive number is returned, where the number returned is the 
  number of structures which have non-zero revents fields  (in  other words,
  those  descriptors  with  events or errors reported).  A value of 0 indicates 
  that the call timed out and no file descriptors have been selected'

It says nothing about revents when return value <= 0 - ie .revents is undefined
and you shouldn't rely on any useful information being in there. It certainly
doesn't say that an impl will set .revents to 0 upon failure, or poll_res == 0,
so I'd say this is a dbus bug here.

> I suspect Valgrind is being too conservative here; but given that this is
> a boundary case you could easily ignore - do nothing if poll_res == 0 - and
> given that presumably portability is a goal, maybe you'd want to handle this
> rather than rely on implementations setting .revents to zero in the case where
> poll returns zero.

Yeah, definitely don't want to rely on any .revents values if poll_res == 0.

Regards,
Dan.
-- 
|=-            GPG key: http://www.berrange.com/~dan/gpgkey.txt       -=|
|=-       Perl modules: http://search.cpan.org/~danberr/              -=|
|=-           Projects: http://freshmeat.net/~danielpb/               -=|
|=-   berrange at redhat.com  -  Daniel Berrange  -  dan at berrange.com    -=|
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.freedesktop.org/archives/dbus/attachments/20060914/44cff2b0/attachment.pgp