Just a kick-off, this will take time. Didn't look ahead yet, so I might
still have comments on the semantics later. The course looks good.
15 comments:
This code does not negatively affect existing pci support and existing
pci devices,
This reads like "This can't hurt, please wave through.", please
remove/rephrase.
but means that support for other operating systems still
needs to be cobbled together.
Why not fall back to the existing dev-mem code on other OS'?
What is with these empty comments? Please remove (or fill).
Patch Set #2, Line 356: void *
This makes C's type checking even weaker. Why not declare the parameter
as `struct flashrom_pci_device *`? You could still pass a `void *`; just
not any pointer.
Patch Set #2, Line 361: cleanup
`clean up`
Patch Set #2, Line 367: device->sysfs_path = NULL;
What for? we release it before it could be dereferenced again.
Patch Set #2, Line 404: return NULL;
leaking `pcidev_bdf`
Patch Set #2, Line 410: for (dev = pacc->devices; dev; dev = dev->next)
Please always use braces if multiple lines follow.
Patch Set #2, Line 411: if (pci_filter_match(&filter, dev)) {
Please turn into
if (!pci_filter_match(&filter, dev))
continue;
to save the extra indentation level.
no space before semicolon
Patch Set #2, Line 424: if (name)
Please use a conditional expression or just:
if (!name)
name = "<unknown pciids>";
which should also fix the strdup() call below.
s/card/device/ ? it doesn't have to be a "card"
`- 1` is not necessary
found_dev->domain ?
Patch Set #2, Line 852: #if NEED_PCI == 1
Why guard? are there conflicting alternative definitions?
To view, visit change 29078. To unsubscribe, or for help writing mail filters, visit settings.