[PATCH 2/2] Fix one warning

[Jan Vesely]

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 at scarletmail.rutgers.edu] On Behalf Of Jan Vesely
> Sent: Friday, April 24, 2015 10:30 PM
> To: Zhou, Jammy
> Cc: dri-devel at 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 at 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 at rutgers.edu>

-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20160106/c5694ea9/attachment.sig>