On Thu, 2 Nov 2017 09:53:41 +0200
Pekka Paalanen <[email protected]> wrote:

> On Wed, 1 Nov 2017 15:37:53 +0100
> Quentin Glidic <[email protected]> wrote:
> 
> > On 11/1/17 3:24 PM, Pekka Paalanen wrote:  
> > > From: Pekka Paalanen <[email protected]>
> > > 
> > > setup_tty() function uses the tty argument for choosing the tty/VT only
> > > if wl->new_user (the -u option) is given. If the tty option is given
> > > without -u, it will only be used for misleading error messages.
> > > 
> > > To make it clear to the user that -t without -u does not work the way
> > > one might think, let weston-launch exit with an error in that case.
> > > 
> > > Signed-off-by: Pekka Paalanen <[email protected]>
> > > ---
> > >   libweston/weston-launch.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/libweston/weston-launch.c b/libweston/weston-launch.c
> > > index aa7e0711..bc50de74 100644
> > > --- a/libweston/weston-launch.c
> > > +++ b/libweston/weston-launch.c
> > > @@ -722,6 +722,9 @@ main(int argc, char *argv[])
> > >           if ((argc - optind) > (MAX_ARGV_SIZE - 6))
> > >                   error(1, E2BIG, "Too many arguments to pass to weston");
> > >   
> > > + if (tty && !wl.new_user)
> > > +         error(1, 0, "tty option requires -u option as well.");    
> > 
> > Nit: maybe EINVAL?
> > And maybe "-t/--tty … -u/--user", at least use the same format to make 
> > it clearer?  
> 
> The error message of the first form is:
> 
> weston-launch: tty option requires -u option as well.
> 
> the error message of the second form is:
> 
> weston-launch: -t/--tty option requires -u/--user option as well: Invalid 
> argument
> 
> I don't think the "Invalid argument" makes it better, and almost all
> the other uses of error() that do not stem of a library or system call
> failure use 0 already. Therefore I'll settle for:
> 
> weston-launch: -t/--tty option requires -u/--user option as well
> 
> The E2BIG case shown in the patch context seems to be the exception to
> how error() is being used in weston-launch.c.
> 
> > Anyway, this one is:
> > Reviewed-by: Quentin Glidic <[email protected]>  
> 
> Let me know if your R-b still stands with
> 
>       error(1, 0, "-t/--tty option requires -u/--user option as well");
> 

The three patches pushed:
   3baf9ce7..1a2adfed  master -> master


Thanks,
pq

Attachment: pgpnI24kX6oQu.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to