`expr substr "$list" $len2 1 != " "` different behavior with the GNU implementation

[sylvestre]

reported here: https://bugs.launchpad.net/ubuntu/+source/rust-coreutils/+bug/2112445

#!/bin/sh
list='alpha avr hppa m68k loongarch64 (loong64) microblaze microblazeel or1k rx sh4 sh4eb tricore xtensa xtensaeb'; \
echo $list; \
len2=$((${#list}/2)); echo len2 = $len2; \
len2=$((${#list}/2)); \
if [ $len2 -gt 36 ]; then \
  while expr substr "$list" $len2 1 != " " >/dev/null; do \
    len2=$(($len2+1)); \
    echo $len2; \
  done; \
  list="$(expr substr "$list" 1 $(($len2-1)))\${Newline} $(expr substr "$list" $(($len2+1)) $len2)"; \
fi;

currently displays:

alpha avr hppa m68k loongarch64 (loong64) microblaze microblazeel or1k rx sh4 sh4eb tricore xtensa xtensaeb
len2 = 53
54
55
56
57
[...]

It should display

alpha avr hppa m68k loongarch64 (loong64) microblaze microblazeel or1k rx sh4 sh4eb tricore xtensa xtensaeb
len2 = 53

[tgrez]

Hey, I'd like to implement a fix for this.

So far, I've looked into a possible root cause and created a minimal script reproducing the issue:

if expr substr "ab cd" 3 1 != " "; then echo true; else echo false; fi;

Which gives different results for GNU and coreutils expr substr.

It seems like the issue is in incorrect precedence when parsing, resulting in similar behavior as expr substr "ab cd" 3 (1 != " "). Changing self.parse_expression() to self.parse_simple_expression() in:

    fn parse_simple_expression(&mut self) -> ExprResult<AstNode> {
            ...
            "substr" => {
                let string = self.parse_expression()?;
                let pos = self.parse_expression()?;
                let length = self.parse_expression()?;
                AstNodeInner::Substr {
                    string: Box::new(string),
                    pos: Box::new(pos),
                    length: Box::new(length),
                }
            }

fixes the issue.

Do I have a green light to create a PR with a fix like presented above? Should I also fix it for other commands (match, index, length)?

[sylvestre]

sure, please go ahead :)
don't hesitate to split the changes into different PR (with tests please :)