Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31385 )
Change subject: util/spdtool: Add tool to extract SPD from BLOBs ......................................................................
Patch Set 4:
(8 comments)
Fixed most of the PEP8 errors.
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py File util/spdtool/spdtool.py:
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@1 PS3, Line 1: #!/usr/bin/env python
Starting comments. […]
"/usr/bin/env python" as it's compatible to python2 and python3.
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@91 PS3, Line 91: if self.blob_as_ord(offset) != i: : continue : if not self.verify_match(i, offset): : continue
nit: The multiple "continue"s followed by the return are a bit harder to read than is necessary. […]
Done
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@99 PS3, Line 99: def get_matches(self):
This function does not use "self". […]
Done
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@149 PS3, Line 149: if offset + 0x15c > len(self.blob):
This looks like it should be >=, given that you are indexing self.blob[offset + 0x15c] below. […]
Done
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@155 PS3, Line 155: if offset + 0x141 > len(self.blob):
This should also be >=
Done
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@161 PS3, Line 161: if offset + 0x7d > len(self.blob):
Again here
Done
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@193 PS3, Line 193: offset = 0 : length = 0
Dead code can be deleted.
Done
https://review.coreboot.org/#/c/31385/3/util/spdtool/spdtool.py@212 PS3, Line 212: fn = open(filename, "w")
idiomatic Python is to use a "with" block here to ensure that all variants of Python will properly f […]
Done