[PATCH xserver] modesetting: Allow a DRM fd to be passed on command line with -masterfd

Lyude Paul lyude at redhat.com
Thu Jun 28 20:31:35 UTC 2018


On Thu, 2018-06-28 at 12:39 -0700, Keith Packard wrote:
> Lyude Paul <lyude at redhat.com> writes:
> 
> > Looks good! One nitpick I'm not 100% sure about:
> > > +#define CHECK_FOR_REQUIRED_ARGUMENT() \
> > > +    if (((i + 1) >= argc) || (!argv[i + 1])) { 				
> > > \
> > > +      ErrorF("Required argument to %s not specified\n", argv[i]); 	
> > > \
> > > +      UseMsg(); 							
> > > \
> > > +      FatalError("Required argument to %s not specified\n", argv[i]);	
> > 
> > Is the double printing of "Required argument to %s not specified" here
> > intentional?
> 
> I copied CHECK_FOR_REQUIRED_ARGUMENT from xf86Init.c where it does the
> same thing. I assume this is intended to make sure the user understands
> what error caused the server to exit -- you can see it both before and
> after the long usage message.

Makes sense!

Reviewed-by: Lyude Paul <lyude at redhat.com>
> 
> > > +    if (!xf86PrivsElevated())
> > > +        ErrorF("-masterfd <fd>         use the specified fd as the DRM
> > > master fd\n");
> > 
> > I think it would be a better idea for us to show this argument description
> > unconditionally, along with adding a note about setuid/setgid not being
> > allowed with it
> 
> Sounds good. 
> 
> Here's an updated patch with the usage message change suggested. Thanks
> for reviewing!
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
-- 
Cheers,
	Lyude Paul


More information about the xorg-devel mailing list