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@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
