<div class="gmail_quote">On Fri, Jul 15, 2011 at 4:06 PM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Don't ignore -i if it is specified before -l<br>
Check if image mentioned by -i is present in layout file<br>
Clean up cli_classic.c:<br>
- Consolidate duplicated programmer_shutdown calls<br>
- Kill outdated comments<br>
- Finish parameter checking before -L/-z is executed </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
This patch was inspired by a cleanup patch by Stefan Tauner.<br>
I did not fix up the -i/-l ordering dependency because the new layout<br>
code may have different requirements anyway, and introducing gratuitous<br>
user interface changes would be a bad idea.<br>
Side note: This also reduces the amount of code changes needed for the<br>
logfile patch.<br>
<br>
Signed-off-by: Carl-Daniel Hailfinger <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>><br></blockquote><div><br></div><div>Looks good to me.</div><div><br></div><div>Acked-by: David Hendricks <<a href="mailto:dhendrix@google.com">dhendrix@google.com</a>></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Index: flashrom-cli_classic_cleanup/cli_classic.c<br>
===================================================================<br>
--- flashrom-cli_classic_cleanup/cli_classic.c (Revision 1372)<br>
+++ flashrom-cli_classic_cleanup/cli_classic.c (Arbeitskopie)<br>
@@ -117,6 +117,7 @@<br>
#endif<br>
int operation_specified = 0;<br>
int i;<br>
+ int ret = 0;<br>
<br>
static const char optstring[] = "r:Rw:v:nVEfc:m:l:i:p:Lzh";<br>
static const struct option long_options[] = {<br>
@@ -232,8 +233,14 @@<br>
cli_classic_abort_usage();<br>
break;<br>
case 'i':<br>
+ /* FIXME: -l has to be specified before -i. */<br>
tempstr = strdup(optarg);<br>
- find_romentry(tempstr);<br>
+ if (find_romentry(tempstr)) {<br>
+ fprintf(stderr, "Error: image %s not found in "<br>
+ "layout file or -i specified before "<br>
+ "-l\n", tempstr);<br>
+ cli_classic_abort_usage();<br>
+ }<br>
break;<br>
case 'L':<br>
if (++operation_specified > 1) {<br>
@@ -313,6 +320,11 @@<br>
}<br>
}<br>
<br>
+ if (optind < argc) {<br>
+ fprintf(stderr, "Error: Extra parameter found.\n");<br>
+ cli_classic_abort_usage();<br>
+ }<br>
+<br>
/* FIXME: Print the actions flashrom will take. */<br>
<br>
if (list_supported) {<br>
@@ -327,11 +339,6 @@<br>
}<br>
#endif<br>
<br>
- if (optind < argc) {<br>
- fprintf(stderr, "Error: Extra parameter found.\n");<br>
- cli_classic_abort_usage();<br>
- }<br>
-<br>
#if CONFIG_INTERNAL == 1<br>
if ((programmer != PROGRAMMER_INTERNAL) && (lb_part || lb_vendor)) {<br>
fprintf(stderr, "Error: --mainboard requires the internal "<br>
@@ -360,8 +367,8 @@<br>
<br>
if (programmer_init(pparam)) {<br>
fprintf(stderr, "Error: Programmer initialization failed.\n");<br>
- programmer_shutdown();<br>
- exit(1);<br>
+ ret = 1;<br>
+ goto out_shutdown;<br>
}<br>
<br>
for (i = 0; i < ARRAY_SIZE(flashes); i++) {<br>
@@ -377,8 +384,8 @@<br>
for (i = 0; i < chipcount; i++)<br>
printf(" %s", flashes[i].name);<br>
printf("\nPlease specify which chip to use with the -c <chipname> option.\n");<br>
- programmer_shutdown();<br>
- exit(1);<br>
+ ret = 1;<br>
+ goto out_shutdown;<br>
} else if (!chipcount) {<br>
printf("No EEPROM/flash device found.\n");<br>
if (!force || !chip_to_probe) {<br>
@@ -389,15 +396,14 @@<br>
startchip = probe_flash(0, &flashes[0], 1);<br>
if (startchip == -1) {<br>
printf("Probing for flash chip '%s' failed.\n", chip_to_probe);<br>
- programmer_shutdown();<br>
- exit(1);<br>
+ ret = 1;<br>
+ goto out_shutdown;<br>
}<br>
printf("Please note that forced reads most likely contain garbage.\n");<br>
return read_flash_to_file(&flashes[0], filename);<br>
}<br>
- // FIXME: flash writes stay enabled!<br>
- programmer_shutdown();<br>
- exit(1);<br>
+ ret = 1;<br>
+ goto out_shutdown;<br>
}<br>
<br>
fill_flash = &flashes[0];<br>
@@ -409,22 +415,19 @@<br>
(!force)) {<br>
fprintf(stderr, "Chip is too big for this programmer "<br>
"(-V gives details). Use --force to override.\n");<br>
- programmer_shutdown();<br>
- return 1;<br>
+ ret = 1;<br>
+ goto out_shutdown;<br>
}<br>
<br>
if (!(read_it | write_it | verify_it | erase_it)) {<br>
printf("No operations were specified.\n");<br>
- // FIXME: flash writes stay enabled!<br>
- programmer_shutdown();<br>
- exit(0);<br>
+ goto out_shutdown;<br>
}<br>
<br>
if (!filename && !erase_it) {<br>
printf("Error: No filename specified.\n");<br>
- // FIXME: flash writes stay enabled!<br>
- programmer_shutdown();<br>
- exit(1);<br>
+ ret = 1;<br>
+ goto out_shutdown;<br>
}<br>
<br>
/* Always verify write operations unless -n is used. */<br>
@@ -437,4 +440,8 @@<br>
*/<br>
programmer_delay(100000);<br>
return doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it);<br>
+<br>
+out_shutdown:<br>
+ programmer_shutdown();<br>
+ return ret;<br>
}<br>
<font color="#888888"><br>
<br>
--<br>
<a href="http://www.hailfinger.org/" target="_blank">http://www.hailfinger.org/</a><br>
<br>
<br>
_______________________________________________<br>
flashrom mailing list<br>
<a href="mailto:flashrom@flashrom.org">flashrom@flashrom.org</a><br>
<a href="http://www.flashrom.org/mailman/listinfo/flashrom" target="_blank">http://www.flashrom.org/mailman/listinfo/flashrom</a><br>
</font></blockquote></div><br><br clear="all"><br>-- <br>David Hendricks (dhendrix)<br>Systems Software Engineer, Google Inc.<br>