4 comments:
File src/commonlib/bsd/cbfs_private.c:
Patch Set #11, Line 75: offset + sizeof(mdata.h), todo) != todo)
Line 53 guarantees sizeof(mdata) >= data_offset
Should line 55 return an error then instead of continuing? I am concerned about silently missing the assumption that data_offset will always be <= sizeof(mdata).
File src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h:
Patch Set #11, Line 4: PRIVATE
Thanks for the explanation. […]
I think having a comment about intention that you provided above, Julius, would be helpful. Obviously it doesn't need to be as long as what wrote, but explain the intention. That way we can point people at it if we see violations of the original intent.
Patch Set #11, Line 82: enum cbfs_walk_flags
I much prefer using an enum over a plain unsigned int. […]
It is helpful to provide that extra hint, but an enum tells me that it's a single value of that enumeration -- not a combination of values from the enumeration. I guess I'm in the minority on that, but enum doesn't scream combination of flags.
Patch Set #11, Line 94: Verify the metadata with |master_hash| if provided.
Discarding a const qualifier is a very bad idea, IMHO.
Ya. I wouldn't cast off the const.
To view, visit change 38421. To unsubscribe, or for help writing mail filters, visit settings.