Skip to content

Fix: Add better handling of division by 0 (int and float) for MOD()#2994

Open
santorofer wants to merge 1 commit into
MDSplus:alphafrom
santorofer:tdi_modulus
Open

Fix: Add better handling of division by 0 (int and float) for MOD()#2994
santorofer wants to merge 1 commit into
MDSplus:alphafrom
santorofer:tdi_modulus

Conversation

@santorofer

@santorofer santorofer commented Dec 3, 2025

Copy link
Copy Markdown
Contributor

This is intended to be a discussion, there are several ways this can be handled for integers. Regardless, $ROPRAND is well suited for handling divide by zero errors for floating points.

Option 1: We could make both DIVIDE and MOD return the new TdiDIV_BY_ZERO error when dividing by zero
Option 2: We could make only MOD return this, and leave DIVIDE returning 0
Option 3: Make MOD return 0 as well and not add TdiDIV_BY_ZERO

This moves MOD (mod_float, mod_bin, OperateBin, Tdi3Mod) into TdiDivide.c
This currently adds a new TdiDIV_BY_ZERO error to tdishr_messages.xml (see above)

This is intended to fix the error handling for 0 % 0, which currently segfaults.

- Move relevant MOD functions into TdiDive.c (i.e mod_float, mod_bin, OperateBin, Tdi3Mod)
- Add new DIV_BY_ZERO in tdishr_messages.xml
- Make use of such new error message
- Refactor switch statement
@mwinkel-dev

Copy link
Copy Markdown
Contributor

I've glanced at this PR and it looks good, however I will do a more thorough review later.

Here are some initial comments . . .

  • I favor adding the new DIV_BY_ZERO status code for division even though it might be a breaking change for some customers. However, as per this morning's team meeting, this proposed change merits additional discussion.

  • The $ROPRAND constant (for missing data) should be included in the new TDI documentation. (It is described in the existing Wiki, but not with the other constants.)

@santorofer santorofer self-assigned this Jan 7, 2026

@heidthecamp heidthecamp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over all good. But there was an erroneous white space change

Comment thread tdishr/TdiMath2.c
@joshStillerman

Copy link
Copy Markdown
Contributor

I concur that floating point behavior should remain. It is complicated but useful that operators of $roperand return $roperand.

maybe we should return $roperand regardless of input types and throw a new error when trying to convert $ROPERAND to integer types?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants