Thanks Quentin for the review. Requesting Tom to consider the merge.
On Wed 08 Jul 06:05 PM, Quentin Schulz wrote: > Hi Naveen, > > On 7/8/26 4:33 PM, Naveen Kumar Chaudhary wrote: > > The '/' and '%' arms of the operator switch in do_setexpr() perform > > a / b and a % b directly on the user-supplied ulongs with no check > > that b is non-zero. The consequences are architecture-dependent: > > > > - On x86 and sandbox, integer divide-by-zero raises an exception > > (#DE / SIGFPE) which aborts the shell. > > - On arm64, arm, and RISC-V, the UDIV/SDIV instructions are defined > > to return 0 on a zero divisor and do not trap, so 'setexpr x 10 > > / 0' silently succeeds and stores 0 into the environment > > variable, which is a plausible-looking but garbage result. > > > > Neither behaviour is acceptable for a scripting primitive. Check b > > before dividing and print a clear error, returning failure so > > scripts can detect it. > > > > ++++++ Before Fix ++++++++ > > => setexpr x 0xa / 0 > > => printenv x > > x=0 > > > > ++++++ After Fix +++++++++ > > => setexpr x 0xa / 0 > > Error: division by zero > > => setexpr x 0xa % 0 > > Error: modulo by zero > > => > > Signed-off-by: Naveen Kumar Chaudhary <[email protected]> > > Makes sense to me, though we definitely need new tests for those, c.f. > test/cmd/setexpr.c, so we don't regress in the future. > > Cheers, > Quentin
