New subject: [PATCH 1/7] Introduce and use enum test_state

3 Mar 2012

Am 03.03.2012 21:11 schrieb Stefan Tauner:
...
  Previously boards in the wiki were tagged either as
working or as known
 bad. But we added support to various boards via board enables that were
 then never tested because the owners have not reported back. This can
 now be tagged with NT and is shown appropriately.

 Also, the underlying data structure indicating state was converted from
 macros to an enum while preserving original integer values.

 Because all lines specifying supported boards and laptops were touched
 anyway, this patch also re-indents them.

 ---
 TODO: change other occurrences to use it.  wanted to get feedack first.

 Signed-off-by: Stefan Tauner &lt;stefan.tauner(a)student.tuwien.ac.at&gt;

 diff --git a/print_wiki.c b/print_wiki.c
 index 377154d..9a9cd83 100644
 --- a/print_wiki.c
 +++ b/print_wiki.c
 @@ -136,9 +136,9 @@ static void wiki_helper(const char *devicetype, int cols,
  	const struct board_match *b = board_matches;

  	for (i = 0; boards[i].vendor != NULL; i++) {
 -		if (boards[i].working)
 +		if (boards[i].working == OK)
  			boardcount_good++;
 -		else
 +		if (boards[i].working == BAD)
  			boardcount_bad++; 
You could replace that construct with a switch(), and it might make
sense to count untested boards as well.

...
   	}

 @@ -171,7 +171,8 @@ static void wiki_helper(const char *devicetype, int cols,
  		       b[k].lb_vendor ? b[k].lb_vendor : "",
  		       b[k].lb_vendor ? ":" : "",
  		       b[k].lb_vendor ? b[k].lb_part : "",
 -		       (boards[i].working) ? "OK" : "No");
 +		       (boards[i].working == OK) ? "OK" :
 +		       (boards[i].working == NT) ? "?3" : "No"); 
The ?3 looks odd, but I assume you tested it.

...

  		if (boards[i].note) {
  			printf("<sup>%d</sup>\n", num_notes + 1);
 diff --git a/print.c b/print.c
 index 1fdeac7..544a846 100644
 --- a/print.c
 +++ b/print.c
 @@ -545,428 +545,427 @@ void print_supported(void)
 [...]
 -	B("ZOTAC",	"Fusion-ITX WiFi (FUSION350-A-E)", 1, NULL, NULL),
 -	B("ZOTAC",	"GeForce 8200",		1,
"http://pden.zotac.com/index.php?page=shop.product_details&product_id=129&category_id=92",
NULL),
 -	B("ZOTAC",	"H67-ITX WiFi (H67ITX-C-E)", 0, NULL, "Probing
works (Winbond W25Q32, 4096 kB, SPI), but parts of the flash are problematic: descriptor
is r/o (conforming to ICH reqs), ME region is locked."),
 -	B("ZOTAC",	"ZBOX HD-ID11",		1,
"http://pdde.zotac.com/index.php?page=shop.product_details&product_id=240&category_id=75",
NULL),

+	B("A-Trend",	"ATC-6220",				OK,	"http://www.motherboard.cz/mb/atrend/atc6220.htm",
NULL),

+	B("abit",	"A-S78H",				OK,	"http://www.abit.com.tw/page/en/motherboard/motherboard_detail.php?pMODEL_NAME=A-S78H&fMTYPE=Socket+AM2",
NULL),

+	B("abit",	"AN-M2",				OK,	"http://www.abit.com.tw/page/en/motherboard/motherboard_detail.php?DEFTITLE=Y&fMTYPE=Socket%20AM2&pMODEL_NAME=AN-M2",
NULL),

+	B("abit",	"AV8",					OK,	"http://www.abit.com.tw/page/en/motherboard/motherboard_detail.php?DEFTITLE=Y&fMTYPE=Socket%20939&pMODEL_NAME=AV8",
NULL), 
Is it possible that this patch made the board table one or two tabs
wider? The patch looks like that, and while I agree that some
files/sections should not have line length limits, adding another 16
columns of whitespace is something I'd like to avoid.

print_wiki related code is something I rarely touch (except for
programmer additions), so please don't expect in-depth reviews from me.
A cursory review suggests that the patch at least doesn't make the code
worse and I don't have any strong feelings about this code. If you feel
this patch is beneficial, I can send a weak Acked-by, more like Meh-by.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

Re: [flashrom] [PATCH 1/7] Introduce and use enum test_state