Bug 32264 - Ineffective error condition in _dbus_pipe_close
Summary: Ineffective error condition in _dbus_pipe_close
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.4.x
Hardware: Other All
: medium major
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
URL:
Whiteboard: r+ from walters
Keywords: patch
Depends on:
Blocks:
 
Reported: 2010-12-09 06:26 UTC by Christian Dywan
Modified: 2011-01-05 04:52 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Fix error condition in _dbus_pipe_close (593 bytes, patch)
2010-12-09 06:27 UTC, Christian Dywan
Details | Splinter Review
Fix error condition in _dbus_pipe_close #2 (594 bytes, patch)
2010-12-09 06:56 UTC, Christian Dywan
Details | Splinter Review

Description Christian Dywan 2010-12-09 06:26:12 UTC
The check for _dbus_close in _dbus_pipe_close is bogus, as it checks for < 0, whereas a boolean is returned.
Comment 1 Christian Dywan 2010-12-09 06:27:05 UTC
Created attachment 40952 [details] [review]
Fix error condition in _dbus_pipe_close
Comment 2 Colin Walters 2010-12-09 06:44:13 UTC
Review of attachment 40952 [details] [review]:

::: dbus/dbus-pipe-unix.c
@@ +72,3 @@
                    DBusError        *error)
 {
+  if (_dbus_close (pipe->fd_or_handle, error))

Shouldn't there be a "!" in there?
Comment 3 Colin Walters 2010-12-09 06:44:13 UTC
Review of attachment 40952 [details] [review]:

::: dbus/dbus-pipe-unix.c
@@ +72,3 @@
                    DBusError        *error)
 {
+  if (_dbus_close (pipe->fd_or_handle, error))

Shouldn't there be a "!" in there?
Comment 4 Colin Walters 2010-12-09 06:44:21 UTC
Review of attachment 40952 [details] [review]:

::: dbus/dbus-pipe-unix.c
@@ +72,3 @@
                    DBusError        *error)
 {
+  if (_dbus_close (pipe->fd_or_handle, error))

Shouldn't there be a "!" in there?
Comment 5 Christian Dywan 2010-12-09 06:56:35 UTC
Created attachment 40954 [details] [review]
Fix error condition in _dbus_pipe_close #2

Oops, indeed.
Comment 6 Colin Walters 2010-12-09 06:57:52 UTC
Review of attachment 40954 [details] [review]:

Looks good, thanks!  (And sorry for the duplicate reviews, splinter was having a bad day)
Comment 7 Simon McVittie 2011-01-05 04:52:02 UTC
I applied this to master, it'll be in 1.4.4 (or possibly 1.5.0).


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.