-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(utk): Add CLI flag -erase-polarity #358
Conversation
Codecov Report
@@ Coverage Diff @@
## master #358 +/- ##
=======================================
Coverage 41.69% 41.69%
=======================================
Files 110 110
Lines 7642 7642
=======================================
Hits 3186 3186
Misses 3877 3877
Partials 579 579 Continue to review full report at Codecov.
|
cmds/utk/utk.go
Outdated
|
||
switch { | ||
case *erasePolarityFlag == "": | ||
case strings.HasPrefix(*erasePolarityFlag, "0x"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to do this. If you do strconv.ParseUint with a base of 0, it will detect the base automatically and include support for stuff like Octal (not that I'm sure anyone should attempt to supply it in Octal :P)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. Didn't know that. Thanks, will fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
cmds/utk/utk.go
Outdated
if err != nil { | ||
return config{}, nil, fmt.Errorf("unable to parse erase polarity '%s': %w", (*erasePolarityFlag)[2:], err) | ||
} | ||
cfg.ErasePolarity = &[]uint8{uint8(erasePolarity)}[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there value in ensuring the erase polarity is either 0 or 0xFF? with this we could supply 0xDE if we wanted to. I'm ok either way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the polarity is invalid, it will panic here:
https://github.com/linuxboot/fiano/pull/358/files#diff-239a5539d7bbec3894ab256b4b20075436aa19b06b5f49995786e36163bdd837R109-R111
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok!
72d0b42
to
bd4db7b
Compare
ITS: #356 Signed-off-by: Dmitrii Okunev <[email protected]>
0a8e8d1
to
f7df9fe
Compare
ITS: #356
Before
After