On Mon, 2015-04-27 at 01:40 +0000, Zhou, Jammy wrote:
Thanks for sharing the background. For [0], you mentioned that get_perms may return -1 in some cases for the group, can you help indicate which case it is?
The one i found is in xserver: dri_drm_get_perms (hw/xfree86/dri/dri.c:759) copies values from xf86ConfigDRI.
xf86configDRI is initialized in configDRI(hw/xfree86/common/xf86Config.c:2166). However, the default value if the DRI section is not present or does not contain group setting is -1.
it looks like it relies on libdrm to fall back to default in that case, and it looks like that path currently broken
I don't claim to fully understand what that old code is doing/supposed to do, but scanning through it suggests that negative values are legal way to report errors/undefined values.
there might be other users as well
jan
Since the drmSetServerInfo is seldom used, maybe we can just do the 'int' cast at this moment. I will send v2 for this.
Regards, Jammy
-----Original Message----- From: Jan Vesely [mailto:jv356@scarletmail.rutgers.edu] On Behalf Of Jan Vesely Sent: Friday, April 24, 2015 10:30 PM To: Zhou, Jammy Cc: dri-devel@lists.freedesktop.org; Min, Frank Subject: Re: [PATCH 2/2] Fix one warning
On Fri, 2015-04-24 at 11:44 +0800, Jammy Zhou wrote:
xf86drm.c:356:2: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits] group = (serv_group >= 0) ? serv_group : DRM_DEV_GID; ^
Signed-off-by: Jammy Zhou Jammy.Zhou@amd.com
xf86drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xf86drm.c b/xf86drm.c index 4d67861..fbda2aa 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -353,7 +353,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type) }
if (drm_server_info) {
- group = (serv_group >= 0) ? serv_group : DRM_DEV_GID;
- group = serv_group;
I don't think this change is correct. get_perms can return errors as negative values. I found that xserver does, see [0] for my take on fixing this, as well as Emil's response [1].
I think changing the condition to: ((int)serv_group >= 0)
should be ok(I don't think there are systems with GID_MAX greater than 2^31-1), but I never bothered sending v2.
jan
[0]http://lists.freedesktop.org/archives/dri-devel/2015-February/077276.html [1]http://lists.freedesktop.org/archives/dri-devel/2015-February/078171.html
chown_check_return(buf, user, group); chmod(buf, devmode); }
-- Jan Vesely jan.vesely@rutgers.edu