Bug 17061

Summary: [PATCH] dbus-1.2.3 does not work on platforms that do not implement _SC_GETPW_R_SIZE_MAX
Product: dbus Reporter: Joe Marcus Clarke <marcus>
Component: coreAssignee: Havoc Pennington <hp>
Status: RESOLVED FIXED QA Contact: John (J5) Palmieri <johnp>
Severity: major    
Priority: medium CC: walters
Version: 1.2.x   
Hardware: Other   
OS: FreeBSD   
Whiteboard:
i915 platform: i915 features:
Attachments: Properly check to see that sysconf returns a positive value

Description Joe Marcus Clarke 2008-08-10 12:54:05 UTC
Created attachment 18196 [details] [review]
Properly check to see that sysconf returns a positive value

If sysconf (_SC_GETPW_R_SIZE_MAX) returns -1, then dbus will not initialize.  The reason for this is that the return from sysconf is stored as a size_t which is unsigned.  Therefore, the check to see if the value is <= 0 is useless.  Attached is a patch which corrects the two locations of this problem.
Comment 1 Havoc Pennington 2008-08-10 14:10:19 UTC
Comment on attachment 18196 [details] [review]
Properly check to see that sysconf returns a positive value

It looks like sysconf() is documented to return long, not size_t, so wouldn't the correct patch change "buflen" to long instead of casting?
Comment 2 Joe Marcus Clarke 2008-08-10 14:45:04 UTC
I thought about that, but getpwuid_r takes a size_t argument, so some casting would have to be done.  The approach I used produced no compiler warnings on i386 and amd64 platforms.
Comment 3 Joe Marcus Clarke 2008-08-10 17:25:33 UTC
(In reply to comment #2)
> I thought about that, but getpwuid_r takes a size_t argument, so some casting
> would have to be done.  The approach I used produced no compiler warnings on
> i386 and amd64 platforms.
> 

Another approach would be to do:

if (buflen == (size_t) -1)
    buflen = 1024;

This doesn't handle the 0 case, but I don't think that's really valid.  The return from sysconf() should either be positive or -1.
Comment 4 Colin Walters 2008-09-04 19:15:41 UTC
Doesn't seem to be an elegant solution here, just going with the first patch.  A note: I'm a big fan of code archaeology using Bugzilla links, so I changed your patch to add comment with a link back to this bug.

commit 3564e5cbe4d9c0538d6eb519904ef0befab39d75
Author: Joe Marcus Clarke <marcus@freedesktop.org>
Date:   Thu Sep 4 22:13:30 2008 -0400

    Bug 17061: Handle error return from sysconf correctly
    
    	* dbus/dbus-sysdeps-unix.c:
    	* dbus/dbus-sysdeps-util-unix.c: Cast return
    	from sysconf temporarily so we actually see
    	-1.
    
    Signed-off-by: Colin Walters <walters@verbum.org>

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 3f963bc..24a3774 100644
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -1493,7 +1493,11 @@ fill_user_info (DBusUserInfo       *info,
     /* retrieve maximum needed size for buf */
     buflen = sysconf (_SC_GETPW_R_SIZE_MAX);
 
-    if (buflen <= 0)
+    /* sysconf actually returns a long, but everything else expects size_t,
+     * so just recast here.
+     * https://bugs.freedesktop.org/show_bug.cgi?id=17061
+     */
+    if ((long) buflen <= 0)
       buflen = 1024;
 
     result = -1;
diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c
index 55eb934..0343a90 100644
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -836,7 +836,11 @@ fill_group_info (DBusGroupInfo    *info,
     /* retrieve maximum needed size for buf */
     buflen = sysconf (_SC_GETGR_R_SIZE_MAX);
 
-    if (buflen <= 0)
+    /* sysconf actually returns a long, but everything else expects size_t,
+     * so just recast here.
+     * https://bugs.freedesktop.org/show_bug.cgi?id=17061
+     */
+    if ((long) buflen <= 0)
       buflen = 1024;
 
     result = -1;

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.