Re: [coreboot] #92: Add --version option

[Ulf Jordan]

Hello Bernhard!

Comments inline.

On Sun, 20 Jan 2008, Bernhard Walle wrote:
> coreboot  [2008-01-20 12:20]:
> > #92: Add --version option
> > ---------------------------------+------------------------------------------
> >    Reporter:  uwe                |          Owner:  somebody
> >        Type:  defect             |         Status:  new
> >    Priority:  major              |      Milestone:  Enhance the flashrom utility
> >   Component:  flashrom           |        Version:
> >    Keywords:                     |   Dependencies:
> > Patchstatus:  there is no patch  |
> > ---------------------------------+------------------------------------------
> >  Flashrom should have a --version option too, similar to superiotool (based
> >  on svn revision).
>
> Good idea, what about this one:
>
> This patch adds version information to flashrom. Because 'v' and 'V'
> are already in use, the patch uses 'R' (for release) and, of course,
> '--version'.
>
> Signed-off-by: Bernhard Walle
> Index: Makefile
>
> --- Makefile	(Revision 3064)
> +++ Makefile	(Arbeitskopie)
> @@ -28,6 +28,12 @@
>
> all: pciutils dep $(PROGRAM)
>
> +# Set the superiotool version string from the highest revision number
> +# of the checked out superiotool files.

s/superiotool/flashrom/
> +SVNDEF := -D'FLASHROM_VERSION="$(shell svnversion -cn . \
>      | sed -e "s/.*://" -e "s/\([0-9]*\).*/\1/")"'
>
> +CFLAGS += $(SVNDEF)
>
> $(PROGRAM): $(OBJS)
>    $(CC) -o $(PROGRAM) $(OBJS) $(LDFLAGS)
>    $(STRIP) $(STRIP_ARGS) $(PROGRAM)
> Index: flashrom.c
> ===================================================================
> --- flashrom.c	(Revision 3064)
> +++ flashrom.c	(Arbeitskopie)
> @@ -206,11 +206,17 @@

Please insert R in the first line of the usage summary too, like for the 
other options.
>     "   -f | --force:                   force write without checking image\n"
>     "   -l | --layout <file.layout>:    read rom layout from file\n"
>     "   -i | --image <name>:            only flash image name from flash layout\n"
>     "   -R | --version:                 print the version (release)\n"
>       "\n" " If no file is specified, then all that happens"
>   " is that flash info is dumped.\n\n");
> exit(1);
>
> }
>
> +void print_version(void)
> +{
> printf("flashrom r%s\n", FLASHROM_VERSION);
>
> +}
>
> int main(int argc, char *argv[])
> {
>    uint8_t *buf;
> @@ -236,6 +242,7 @@
>    	{"layout", 1, 0, 'l'},
>    	{"image", 1, 0, 'i'},
>    	{"help", 0, 0, 'h'},
> {"version", 0, 0, 'R'},
> {0, 0, 0, 0}
> };
>
> @@ -253,7 +260,7 @@
>    }
>
> setbuf(stdout, NULL);
> while ((opt = getopt_long(argc, argv, "rwvVEfc:s:e:m:l:i:h",
> while ((opt = getopt_long(argc, argv, "rRwvVEfc:s:e:m:l:i:h",
> 		  long_options, &option_index)) != EOF) {
> switch (opt) {
> case 'r':
>
> @@ -306,6 +313,9 @@
>    		tempstr = strdup(optarg);
>    		find_romentry(tempstr);
>    		break;
> case 'R':
> 	print_version();
> 	exit(0);

break;
> case 'h':
> default:
> 	usage(argv[0]);

Apart from the comments above the patch looks very nice.

Please repost updated patch as attachment (avoids white-space breakage in 
mailclients).

/ulf