18 comments:
Patch Set #6, Line 11: no dependency on size_t
I don't understand the point of this line item. API can freely use size_t as it shouldn't matter.
Ack
Patch Set #16, Line 15: UNTESTED.
Should be tested.
Done
File src/drivers/smmstore/store.c:
Patch Set #6, Line 90: uint32_t
I don't understand the point of this change. There's not reason one can't use ssize_t or size_t.
Ack
Patch Set #6, Line 105: bufsize
I don't like the change of the type, but this is a nonsense check now that bufsize is pointing to an […]
Done
you open coded 1 here, but didn't below w/ nul. I think we should be consistent.
Done
Patch Set #6, Line 230: return 0;
We're just always successful it seems?
Done
File src/drivers/smmstore/store.c:
The interface is defined above and the function calling it also uses plain numbers.
Ack
CB_SUCCESS (other instances as well?)
Ack
File src/drivers/smmstore/store.c:
Patch Set #9, Line 227: if (rdev_writeat(&store, &nul, end, sizeof(nul)) != sizeof(nul)) {
Would it be better to have the equivalent of FILE such that the offset is tracked and bumped when re […]
And an equivalent of fseek etc? That's getting complex fast and I'd prefer to see more places where it might be useful before we go there. But definitely something to keep in mind if this pattern proliferates.
File src/drivers/smmstore/store.c:
Patch Set #12, Line 58: struct region_device *rdev;
This is just a pointer that is uninitialized and isn't pointing to a valid object.
Done
File src/drivers/smmstore/store.c:
Patch Set #13, Line 60: rdev.region
*region_device_region(&rdev);
Done
Patch Set #13, Line 87: store_rdev
This change seems like quite the short cut. store_rdev only is used once below. […]
Done
Patch Set #13, Line 211: data_sz
I don't understand why one is passing in data_sz scan_end() can obtain it directly since 'store' is […]
Done
Patch Set #13, Line 219: &store) < 0) {
You already have a writable rdev by way of using incoherent_rdev in lookup_store().
Done
This should be 0. The rdev_chain() tracks the window of the region it is operating on. […]
Done
File src/drivers/smmstore/store.c:
Patch Set #15, Line 136: ssize_t *end
There's no need for this.
Done
you can do this here: […]
Done
And this would be 0 once scan_end() has positioned the region device correctly. […]
Done
To view, visit change 30432. To unsubscribe, or for help writing mail filters, visit settings.