[bugfix] rounding exception #36
Conversation
srecord/arglex/tool/get_number.cc
Outdated
| value /= multiple; | ||
| value *= multiple; | ||
| break; | ||
|
|
||
| case token_round_up: | ||
| token_next(); | ||
| multiple = get_number("-round-up"); | ||
| if(multiple == 0) |
There was a problem hiding this comment.
Add space between if and (
srecord/arglex/tool/get_number.cc
Outdated
| value = (value + multiple - 1) / multiple; | ||
| value *= multiple; | ||
| break; | ||
|
|
||
| case token_round_nearest: | ||
| token_next(); | ||
| multiple = get_number("-round-nearest"); | ||
| if(multiple == 0) |
There was a problem hiding this comment.
Add space between if and (
srecord/arglex/tool/get_number.cc
Outdated
| value = (value + multiple - 1) / multiple; | ||
| value *= multiple; | ||
| break; | ||
|
|
||
| case token_round_nearest: | ||
| token_next(); | ||
| multiple = get_number("-round-nearest"); | ||
| if(multiple == 0) | ||
| { | ||
| fatal_error ("-Round_Nearest value must not be 0"); |
There was a problem hiding this comment.
Remove space between fatal_error and (
srecord/arglex/tool/get_number.cc
Outdated
| value /= multiple; | ||
| value *= multiple; | ||
| break; | ||
|
|
||
| case token_round_up: | ||
| token_next(); | ||
| multiple = get_number("-round-up"); | ||
| if(multiple == 0) | ||
| { | ||
| fatal_error ("-Round_Up value must not be 0"); |
There was a problem hiding this comment.
Remove space between fatal_error and (
|
Still getting used to GH PR process, BB and GL are my primary GIT web interfaces to use. How GH does things is a bit different than I am used to. |
|
I'm also new to GH. I'm missing to reply to a specific comment (as in GL) instead of posting at the end. |
|
About the MegaLinter issues:
|
|
@oceanofthelost, I've merged #32 from @jtxa (which should encompass #33). If you rebase that should take care of the megalinter checks failing . Thank you both! |
Three tests added, one for each rounding method, which will fail when providing input causing exception.
oceanofthelost
force-pushed
the
bugfix/rounding-exception
branch
from
f94c955 to
2e86337
Compare
|
So close! If you can add your name and github username to doc/dictionaries/names.txt, all should be well. I should have picked that one up earlier. I'm putting together some updates with a getting started guide for new contributors and will definitely add this in there. |
Oh, my username is listed there as well, didn't know before. |
@jtxa added a handy list of project-custom words for cspell which runs as part of the MegaLinter checks. This resulted ina monumental number of typos being fixed. Actually, really nice! But yeah, things like names/handles and technical terms like EPROM and even SRecord itself now get accepted gracefully. He even added all the names for contributors so that doc files don't get rejected. |
|
OK, I wondered how my username landed in the SRecord sources, but now I searched and found it in |
|
@sierrafoxtrot This is my opinion: The extra commits of @oceanofthelost were perfect for reviewing, it can make the process easier. But for the long term history the extra commits are irrelevant in my opinion. Who want to see that there were spelling, formatting or other problems on code, that were never used by anyone yet. In case of a small feature I would squash it to 1 commit, the test are directly related to the implementation. |
If value provided by user for rounding is 0, trigger fatal error signaling 0 is not valid input.
oceanofthelost
force-pushed
the
bugfix/rounding-exception
branch
from
40986a3 to
87d1fd9
Compare
Thanks @jtxa , that make s lot of sense. Clean but preserves the history and effort. I like the approach. Thanks @oceanofthelost for the contribution! This should unlock auto-run for workflows as well (which I can imagine was pretty frustrating) |
Fixes a potential floating point bug (Issue #35) caused by invalid user input.
PR adds three new tests, one per input flag. IN addition, added a check in
get_number.ccwhich checks if user has passed a zero to one of the rounding functions. If zero passed, thenfatal_error()is executed and stops execution of user program.