Well, I can see why a Python aficionado would like that. 8-)
My first reaction is concern for error handling, it seems like it would be pretty easy to specify something with too few or two many arguments. Yeah, the audience is developers and they can probably sort out the exception, but it seems like buying trouble.
The second thing is that my own preferred format is "both" which sort of breaks your model:
out += "%04i x%04X: " % ((i * line_sz), (i * line_sz))
This requires the statement to have two (identical) address arguments in a list. Handling that would require scanning the supplied format, and while I'm sure it's computable, it's more work than I'm interested in, and this flexibility doesn't get me very excited.
If you're OK with letting the user deal with whatever exception they might cause, I could see handling the case of "not decimal or hex or both" by assuming one address argument and just pasting whatever the user supplies for the format.
It doesn't solve any problem I have, but I think it's minimal work and I could do that. I.e.
Eliminate this:
- elif (addrfmt != "decimal" and addrfmt != "hex" and addrfmt != "both"):
print "Invalid hexdump_addrfmt value %s. Using decimal." % addrfmtaddrfmt = "decimal"
and change the print statements to:
...
elif addrfmt == "both":out += "%04i x%04X: " % ((i * line_sz), (i * line_sz))elif addrfmt == "decimal":out += "%04i: " % (i * line_sz)
else out += addrfmt % (i * line_sz)
Thoughts?
-dan
On Jul 15, 2014, at 5:30 PM, Dan Smith - dsmith@danplanet.com wrote:
try:
addrfmt = CONF.get("hexdump_addrfmt", "developer")except Exception:
addrfmt = "decimal"if addrfmt == None:
addrfmt = "decimal"elif (addrfmt != "decimal" and addrfmt != "hex" and addrfmt != "both"):
print "Invalid hexdump_addrfmt value %s. Using decimal." % addrfmtaddrfmt = "decimal"for i in range(0, (len(data)/line_sz)):
out += "%03i: " % (i * line_sz)
if addrfmt == "hex":out += "x%04X: " % (i * line_sz)elif addrfmt == "both":out += "%04i x%04X: " % ((i * line_sz), (i * line_sz))else:out += "%04i: " % (i * line_sz)Just a thought...
Instead of having them choose from a few different symbolic choices, why not just let them specify the format themselves? So in the config:
hexdump_addrfmt = %0000x
and in the code:
fmt = CONF.get("hexdump_addrfmt", "developer") . . . thing = fmt % addr
I can just see someone coming along and wanting octal or something and if we just let them specify the format, we can avoid adding new symbols. What do you think?
--Dan