Hello Bernhard!
Comments inline.
On Sun, 20 Jan 2008, Bernhard Walle wrote:
> * coreboot <[EMAIL PROTECTED]> [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 <[EMAIL PROTECTED]>
>
>
> 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
--
coreboot mailing list
coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot
