[PATCH] Clean up main loop

[Carl-Daniel Hailfinger]

To prepare for libflashrom I wanted to make the main loop more readable
and more correct and factor out stuff which can be useful in libflashrom.

- Factor out printing of supported devices to print.c.
- Adjust name of wiki printing function to fit the pattern.
- Abort if the user specified --verify and --noverify at the same time.
- Check for extra parameters which don't fit commandline syntax.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-mainloop_cleanup/flash.h
===================================================================
--- flashrom-mainloop_cleanup/flash.h	(Revision 755)
+++ flashrom-mainloop_cleanup/flash.h	(Arbeitskopie)
@@ -338,11 +338,9 @@

/* print.c */
 char *flashbuses_to_text(enum chipbustype bustype);
-void print_supported_chips(void);
-void print_supported_chipsets(void);
-void print_supported_boards(void);
+void print_supported(void);
 void print_supported_pcidevs(struct pcidev_status *devs);
-void print_wiki_tables(void);
+void print_supported_wiki(void);

/* board_enable.c */
 void w836xx_ext_enter(uint16_t port);
Index: flashrom-mainloop_cleanup/flashrom.c
===================================================================
--- flashrom-mainloop_cleanup/flashrom.c	(Revision 755)
+++ flashrom-mainloop_cleanup/flashrom.c	(Arbeitskopie)
@@ -801,14 +801,25 @@
    		write_it = 1;
    		break;
    	case 'v':
+			//FIXME: gracefully handle superfluous -v
    		if (++operation_specified > 1) {
    			fprintf(stderr, "More than one operation "
    				"specified. Aborting.\n");
    			exit(1);
    		}
+			if (dont_verify_it) {
+				fprintf(stderr, "--verify and --noverify are"
+					"mutually exclusive. Aborting.\n");
+				exit(1);
+			}
    		verify_it = 1;
    		break;
    	case 'n':
+			if (verify_it) {
+				fprintf(stderr, "--verify and --noverify are"
+					"mutually exclusive. Aborting.\n");
+				exit(1);
+			}
    		dont_verify_it = 1;
    		break;
    	case 'c':
@@ -896,29 +907,13 @@
    }

if (list_supported) {
-		print_supported_chips();
-		print_supported_chipsets();
-		print_supported_boards();
-		printf("\nSupported PCI devices flashrom can use "
-		       "as programmer:\n\n");
-#if NIC3COM_SUPPORT == 1
-		print_supported_pcidevs(nics_3com);
-#endif
-#if GFXNVIDIA_SUPPORT == 1
-		print_supported_pcidevs(gfx_nvidia);
-#endif
-#if DRKAISER_SUPPORT == 1
-		print_supported_pcidevs(drkaiser_pcidev);
-#endif
-#if SATASII_SUPPORT == 1
-		print_supported_pcidevs(satas_sii);
-#endif
+		print_supported();
    	exit(0);
    }

#if PRINT_WIKI_SUPPORT == 1
    if (list_supported_wiki) {
-		print_wiki_tables();
+		print_supported_wiki();
    	exit(0);
    }
 #endif
@@ -930,6 +925,11 @@

if (optind < argc)
    	filename = argv[optind++];
+	
+	if (optind < argc) {
+		printf("Error: Extra parameter found.\n");
+		usage(argv[0]);
+	}

if (programmer_init()) {
    	fprintf(stderr, "Error: Programmer initialization failed.\n");
@@ -1146,7 +1146,7 @@
    	if (write_it)
    		programmer_delay(1000*1000);
    	ret = verify_flash(flash, buf);
-		/* If we tried to write, and now we don't properly verify, we
+		/* If we tried to write, and verification now fails, we
    	 * might have an emergency situation.
    	 */
    	if (ret && write_it)
Index: flashrom-mainloop_cleanup/print_wiki.c
===================================================================
--- flashrom-mainloop_cleanup/print_wiki.c	(Revision 755)
+++ flashrom-mainloop_cleanup/print_wiki.c	(Arbeitskopie)
@@ -535,7 +535,7 @@
    }
 }

-void print_wiki_tables(void)
+void print_supported_wiki(void)
 {
    time_t t = time(NULL);

Index: flashrom-mainloop_cleanup/print.c
===================================================================
--- flashrom-mainloop_cleanup/print.c	(Revision 755)
+++ flashrom-mainloop_cleanup/print.c	(Arbeitskopie)
@@ -222,7 +222,28 @@
    	"Laptops which have been verified to NOT work yet");
 }

+void print_supported(void)
+{
+		print_supported_chips();
+		print_supported_chipsets();
+		print_supported_boards();
+		printf("\nSupported PCI devices flashrom can use "
+		       "as programmer:\n\n");
+#if NIC3COM_SUPPORT == 1
+		print_supported_pcidevs(nics_3com);
+#endif
+#if GFXNVIDIA_SUPPORT == 1
+		print_supported_pcidevs(gfx_nvidia);
+#endif
+#if DRKAISER_SUPPORT == 1
+		print_supported_pcidevs(drkaiser_pcidev);
+#endif
+#if SATASII_SUPPORT == 1
+		print_supported_pcidevs(satas_sii);
+#endif
+}

+
 /* Please keep this list alphabetically ordered by vendor/board. */
 const struct board_info boards_ok[] = {
    /* Verified working boards that don't need write-enables. */

[Carl-Daniel Hailfinger]

On 31.10.2009 03:23, Carl-Daniel Hailfinger wrote:
> To prepare for libflashrom I wanted to make the main loop more readable
> and more correct and factor out stuff which can be useful in libflashrom.
> Factor out printing of supported devices to print.c.
> Adjust name of wiki printing function to fit the pattern.
> Abort if the user specified --verify and --noverify at the same time.
> Check for extra parameters which don't fit commandline syntax.
>
> Signed-off-by: Carl-Daniel Hailfinger

Ack please?
I have a patch on top of this which checks if the chip was modified at
all in case of failing write/erase. Stefan, you already sent out a
preemptive ack for that check-if-anything-changed patch, but this needs
to be applied first.

Regards,
Carl-Daniel

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> To prepare for libflashrom I wanted to make the main loop more readable
> and more correct and factor out stuff which can be useful in libflashrom.
> Factor out printing of supported devices to print.c.
> Adjust name of wiki printing function to fit the pattern.
> Abort if the user specified --verify and --noverify at the same time.
> Check for extra parameters which don't fit commandline syntax.
>
> Signed-off-by: Carl-Daniel Hailfinger

Acked-by: Stefan Reinauer
> Index: flashrom-mainloop_cleanup/flash.h
>
> --- flashrom-mainloop_cleanup/flash.h	(Revision 755)
> +++ flashrom-mainloop_cleanup/flash.h	(Arbeitskopie)
> @@ -338,11 +338,9 @@
>
> /* print.c */
>  char *flashbuses_to_text(enum chipbustype bustype);
> -void print_supported_chips(void);
> -void print_supported_chipsets(void);
> -void print_supported_boards(void);
> +void print_supported(void);
>  void print_supported_pcidevs(struct pcidev_status *devs);
> -void print_wiki_tables(void);
> +void print_supported_wiki(void);
>
> /* board_enable.c */
>  void w836xx_ext_enter(uint16_t port);
> Index: flashrom-mainloop_cleanup/flashrom.c
> ===================================================================
> --- flashrom-mainloop_cleanup/flashrom.c	(Revision 755)
> +++ flashrom-mainloop_cleanup/flashrom.c	(Arbeitskopie)
> @@ -801,14 +801,25 @@
>    		write_it = 1;
>    		break;
>    	case 'v':
> 	//FIXME: gracefully handle superfluous -v
> if (++operation_specified > 1) {
> 	fprintf(stderr, "More than one operation "
> 		"specified. Aborting.\n");
> 	exit(1);
> }
> 	if (dont_verify_it) {
> 		fprintf(stderr, "--verify and --noverify are"
> 			"mutually exclusive. Aborting.\n");
> 		exit(1);
> 	}
> verify_it = 1;
> break;
> case 'n':
> 	if (verify_it) {
> 		fprintf(stderr, "--verify and --noverify are"
> 			"mutually exclusive. Aborting.\n");
> 		exit(1);
> 	}
> dont_verify_it = 1;
> break;
> case 'c':
>
> @@ -896,29 +907,13 @@
>    }
>
> if (list_supported) {
> print_supported_chips();
> print_supported_chipsets();
> print_supported_boards();
> printf("\nSupported PCI devices flashrom can use "
>        "as programmer:\n\n");
>
> -#if NIC3COM_SUPPORT == 1
> print_supported_pcidevs(nics_3com);
>
> -#endif
> -#if GFXNVIDIA_SUPPORT == 1
> print_supported_pcidevs(gfx_nvidia);
>
> -#endif
> -#if DRKAISER_SUPPORT == 1
> print_supported_pcidevs(drkaiser_pcidev);
>
> -#endif
> -#if SATASII_SUPPORT == 1
> print_supported_pcidevs(satas_sii);
>
> -#endif
> print_supported();
> exit(0);
> }
>
> #if PRINT_WIKI_SUPPORT == 1
>    if (list_supported_wiki) {
> print_wiki_tables();
> print_supported_wiki();
> exit(0);
> }
>
> #endif
> @@ -930,6 +925,11 @@
>
> if (optind < argc)
>    	filename = argv[optind++];
>
> if (optind < argc) {
> printf("Error: Extra parameter found.\n");
> usage(argv[0]);
>
> }
>
> if (programmer_init()) {
> fprintf(stderr, "Error: Programmer initialization failed.\n");
>
> @@ -1146,7 +1146,7 @@
>    	if (write_it)
>    		programmer_delay(1000*1000);
>    	ret = verify_flash(flash, buf);
> /* If we tried to write, and now we don't properly verify, we
> /* If we tried to write, and verification now fails, we
> might have an emergency situation.
> */
> if (ret && write_it)
> Index: flashrom-mainloop_cleanup/print_wiki.c
>
> --- flashrom-mainloop_cleanup/print_wiki.c	(Revision 755)
> +++ flashrom-mainloop_cleanup/print_wiki.c	(Arbeitskopie)
> @@ -535,7 +535,7 @@
>    }
>  }
>
> -void print_wiki_tables(void)
> +void print_supported_wiki(void)
>  {
>    time_t t = time(NULL);
> Index: flashrom-mainloop_cleanup/print.c
>
> --- flashrom-mainloop_cleanup/print.c	(Revision 755)
> +++ flashrom-mainloop_cleanup/print.c	(Arbeitskopie)
> @@ -222,7 +222,28 @@
>    	"Laptops which have been verified to NOT work yet");
>  }
>
> +void print_supported(void)
> +{
> print_supported_chips();
> print_supported_chipsets();
> print_supported_boards();
> printf("\nSupported PCI devices flashrom can use "
>        "as programmer:\n\n");
>
> +#if NIC3COM_SUPPORT == 1
> print_supported_pcidevs(nics_3com);
>
> +#endif
> +#if GFXNVIDIA_SUPPORT == 1
> print_supported_pcidevs(gfx_nvidia);
>
> +#endif
> +#if DRKAISER_SUPPORT == 1
> print_supported_pcidevs(drkaiser_pcidev);
>
> +#endif
> +#if SATASII_SUPPORT == 1
> print_supported_pcidevs(satas_sii);
>
> +#endif
> +}
>
> /* Please keep this list alphabetically ordered by vendor/board. */
>  const struct board_info boards_ok[] = {
>    /* Verified working boards that don't need write-enables. */

[Carl-Daniel Hailfinger]

On 17.11.2009 10:08, Stefan Reinauer wrote:
> Acked-by: Stefan Reinauer

Thanks, r766.

Regards,
Carl-Daniel