Fix division by zero #15152
Fix division by zero #15152
Conversation
| @@ -165,20 +165,13 @@ LanguageNameInfo ScintillaEditView::_langNameInfoArray[L_EXTERNAL + 1] = { | |||
|
|
|||
| int getNbDigits(int aNum, int base) | |||
| { | |||
| int nbChiffre = 1; | |||
| int diviseur = base; | |||
| int nbChiffre = 0; | |||
There was a problem hiding this comment.
Why not take the opportunity to convert the code to English at this time?
Suggest nbChiffre become nbDigits.
There was a problem hiding this comment.
You are right. As this is my first contribution, I wanted to change as little as possible. I found more bugs, so I'll know better next time.
There was a problem hiding this comment.
Renamed identifier as suggested
Fixed author
There was a problem hiding this comment.
@donho Hopefully it is ok for me to make the suggestion to convert French to English in the code. I know that when I submitted some Column Editor code changes previously, I did such language updates and it was accepted...
There was a problem hiding this comment.
@alankilborn @cddiffz
Yes, it's good.
I did it (using inconsistently French word) because both are not my mother tongue.
However, the code should be done by a international language for the sake that everyone can understand so everyone can maintain it.
| { | ||
| int result = aNum / diviseur; |
There was a problem hiding this comment.
@cddiffz Did the division by zero happen here?
There was a problem hiding this comment.
Yes, the divisor can become zero due to an arithmetic overflow.
There was a problem hiding this comment.
ok, I see, because also in the new code a div by zero is possible if the provided base parameter is already zero by the caller. Maybe an additional check for that makes sense also currently just base 10, 16, 8, 2 is in use. But there is no description that just these bases could be used.
There was a problem hiding this comment.
The actual calling code first sets the base to 10, then possibly changes it to either 2, 8 or 16. If zero were passed as the base, the calling code would be flawed anyway, wouldn't it? Bases smaller than 2 are unusable for positional numeral systems. There is no point in trying to catch them here.
| ++nbDigits; | ||
| aNum /= base; | ||
| } while (aNum != 0); | ||
| if (base == 16 && nbDigits % 2 != 0) |
There was a problem hiding this comment.
@cddiffz For input aNum = 0 and base =16 this would result in nbDigits = 1 from do-while above already here and as 1%2 = 1 != 0 would result in another increment to nbDigits.
There was a problem hiding this comment.
But that was already the case before. I didn't change the semantics at all, just fixed the bug.
There was a problem hiding this comment.
Yes, that is correct also it seems to be strange.
There was a problem hiding this comment.
This is another bug for me. With "Format" set to "Hex", e.g. instead of "100", "100 " is inserted because of the rounding to the next even number.
There was a problem hiding this comment.
For input aNum = 0 and base =16 this would result in nbDigits = 1 from do-while above already here and as 1%2 = 1 != 0 would result in another increment to nbDigits.
If aNum is 0 & the base is 16, then nbDigits will be 2: I don't see why it's a bug since we present HEX as 0x00.
instead of "100", "100 " is inserted because of the rounding to the next even number.
I don't quite follow you. Could you provide step by step instructions to reproduce the bug please?
Fix #15144