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@xxxxxxxxxxxxxxxxxxxxxxx] On Behalf Of Jan Vesely > Sent: Friday, April 24, 2015 10:30 PM > To: Zhou, Jammy > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; 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@xxxxxxx> > > --- > > 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@xxxxxxxxxxx> -- Jan Vesely <jan.vesely@xxxxxxxxxxx>
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
