On Wed, 4 May 2016 18:29:30 +0200
Armin Krezović <[email protected]> wrote:

> On 02.05.2016 16:14, Pekka Paalanen wrote:
> > On Mon,  2 May 2016 12:27:50 +0200
> > Armin Krezović <[email protected]> wrote:
> >   
> >> This adds a command line argument to print wayland-scanner version.
> >>
> >> It also makes wayland-scanner emit a comment with wayland library
> >> version to every file it generates.
> >>
> >> Signed-off-by: Armin Krezović <[email protected]>
> >> ---
> >>  src/scanner.c | 26 ++++++++++++++++++++++++--
> >>  1 file changed, 24 insertions(+), 2 deletions(-)  
> > 
> > Hi Armin,
> > 
> > this is nice. Some tiny details below, otherwise looking good.
> >   
> >>
> >> diff --git a/src/scanner.c b/src/scanner.c
> >> index 52c07a6..93c4204 100644
> >> --- a/src/scanner.c
> >> +++ b/src/scanner.c
> >> @@ -26,6 +26,7 @@
> >>   */
> >>  
> >>  #include "config.h"
> >> +#include "wayland-version.h"
> >>  
> >>  #include <stdbool.h>
> >>  #include <stdio.h>
> >> @@ -64,12 +65,21 @@ usage(int ret)
> >>                    "headers, server headers, or protocol marshalling 
> >> code.\n\n");
> >>    fprintf(stderr, "options:\n");
> >>    fprintf(stderr, "    -h,  --help                  display this help and 
> >> exit.\n"
> >> +                  "    -v,  --version               print the wayland 
> >> library version that\n"
> >> +                  "                                 the scanner was built 
> >> against.\n"
> >>                    "    -c,  --include-core-only     include the core 
> >> version of the headers,\n"
> >>                    "                                 that is e.g. 
> >> wayland-client-core.h instead\n"
> >>                    "                                 of 
> >> wayland-client.h.\n");
> >>    exit(ret);
> >>  }
> >>  
> >> +static int
> >> +scanner_version(int ret)
> >> +{
> >> +  fprintf(stderr, "wayland-scanner %s\n", WAYLAND_VERSION);
> >> +  exit(ret);
> >> +}  
> > 
> > Usage may be used with error-exit, but this never, so the argument is
> > not necessary. However, the EXIT_SUCCESS in the call is a strong
> > suggestion that this call will exit, so it's good for documentation.
> > Well, that's all just cosmetics. Fine as is, too.
> >   
> 
> See below (##).

> >>  
> >>    if (help)
> >>            usage(EXIT_SUCCESS);
> >> +  else if (version)
> >> +          scanner_version(EXIT_SUCCESS);  
> > 
> > Ok.
> >   
> 
> (##) So, am I going to leave this as it is to be consistent or change it
> as you suggested above?

It's fine either way, and consistency with usage() is not as
important as documenting that the funtion never returns. Having
EXIT_SUCCESS as an argument nicely hints that it may exit(), but so
could "exit" in the function name too.


> The rest of the nitpicks by You and Bryce will be taken care of. Thanks
> for the review.

Cool.

Thanks,
pq

Attachment: pgpMZIxMi3uMt.pgp
Description: OpenPGP digital signature

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

Reply via email to