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?
in xserver: function dr_drm_get_perms (hw/xfree86/dri/dri.c:759) copies previously initialized values of xf86ConfigDRI.
Those values are set in configDRI (hw/xfree86/common/xf86Config.c:2166), which is called from xf86HandleConfigFile (hw/xfree86/common/xf86Config.c:2479) and provided values parsed in xf86readConfigFile(hw/xfree86/parser/read.c:89), group is only set in dri section.
I did not really look into details. to me it looks like -1 is stored in group unless there is a valid DRI section with group assigned name or gid.
I did not look for other users of libdrm that might also use neg values to indicate errors. You might want to ask someone who is more familiar with the design than just reading the code (like I did)
regards, 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