Thanks for the patch! Some comments.
On Sat, Jan 12, 2008 at 06:09:21AM -0500, Robinson Tryon wrote:
I wasn't very sure about what the "wiki links" part of this feature entailed. I included the primary url for superiotool, but perhaps there's something more that needs to be done?
I think there's a page for each supported sio chip with the output from previous superiotool runs. Or there should be. Would be handy to have that address available as well.
Oh, and how should I go about getting an account on Trac?
Stefan should get you hooked up. :)
+void print_fintek_chips(void) {
- char name [] = "Fintek";
- print_vendor_chips(name, reg_table);
+}
Why the extra char[]? The function parameter is const so the name could just be a string constant in the call.
And thinking further I saw that the vendor name is given as a string constant in more places in the files. Perhaps make it a #define near the top of the file? That's a separate patch though, and just a thought.
+void print_vendor_chips(const char *vendor,
const struct superio_registers reg_table[]) {- int i, any_supported_chips = 0;
- printf(" Chips from %s:\n", vendor);
- for (i = 0; /* Nothing */; i++) {
if (reg_table[i].superio_id == EOT)break;
So why not write the break condition in the for statement?
any_supported_chips = 1;print_chip(reg_table[i]);- }
- if (!any_supported_chips)
printf(" None.\n");
any_supported_chips could just check if 0==i, right?
Also - isn't the vendor inclued in the reg_table? Why is it needed at all in this function? Sorry if I'm being slow.
//Peter