#92: Add --version option

[coreboot]

#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).

[Bernhard Walle]

* 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.
+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 @@
         "   -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);
    	case 'h':
    	default:
    		usage(argv[0]);

[Bernhard Walle]

* Bernhard Walle  [2008-01-20 15:45]:
> Signed-off-by: Bernhard Walle

[Stupid abbrevs in vim ;-) -- please use:]
Signed-off-by: Bernhard Walle

Bernhard

[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

[Bernhard Walle]

Hello,

* Ulf Jordan  [2008-01-20 22:48]:
> On Sun, 20 Jan 2008, Bernhard Walle wrote:
> > all: pciutils dep $(PROGRAM)
> >
> > +# Set the superiotool version string from the highest revision number
> > +# of the checked out superiotool files.
>
> s/superiotool/flashrom/

Sorry, copy & paste. :)
> > +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.

Yes, good idea. :)
> > case 'R':
> > 	print_version();
> > 	exit(0);
>
> break;

Well, I don't think that that matters after the program was quit. But
for coding style, done.

Thanks for the review!

Bernhard

[Bernhard Walle]

* Bernhard Walle  [2008-01-20 22:59]:
> Thanks for the review!

It's already too late for me. :-( Now!

[Ulf Jordan]

On Sun, 20 Jan 2008, Bernhard Walle wrote:
> Bernhard Walle  [2008-01-20 22:59]:
> > Thanks for the review!
>
> It's already too late for me. :-( Now!
>
> 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

Acked-by: Ulf Jordan

/ulf

[Uwe Hermann]

On Mon, Jan 21, 2008 at 08:32:24AM +0100, Ulf Jordan wrote:
> > 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
>
> Acked-by: Ulf Jordan

Thanks, committed in r3067. I also updated the manpage with the new option.

Uwe.

[coreboot]

#92: Add --version option
-------------------------+--------------------------------------------------
    Reporter:  uwe       |         Owner:  somebody                    
        Type:  defect    |        Status:  closed                      
    Priority:  major     |     Milestone:  Enhance the flashrom utility
   Component:  flashrom  |       Version:                              
  Resolution:  fixed     |      Keywords:                              
Dependencies:            |   Patchstatus:  patch has been committed    
-------------------------+--------------------------------------------------
Changes (by uwe):

* status:  new => closed
  * patchstatus:  there is no patch => patch has been committed
  * resolution:  => fixed

Comment:

Fixed in r3067.