Skip to content

Commit

Permalink
Fix a 25-year old bug \o/
Browse files Browse the repository at this point in the history
This is a trivial bug, but a bug nevertheless. It's present in the
first commit in this (synthetic) Git repository, and from my
investigations into very old versions - mt-st-0.5 from an old RedHat
version - it was already present in the `stinit.c` version from Apr
11, 1998, which makes it 25 years and 9 days old. This is a new record
for me, which I don't think I'll beat :)

The bug is trivial, in retrospect, and was found just by accident
because with the 1.6 release uploaded in Debian, the new test suite is
run on multiple architectures. While amd64/x86 doesn't trigger this
bug in neither '-O2' nor '-g' build, mips64el, arm64, and s390x do
show the bug in the optimised builds (and only in them) as follows:

amd64:

```
$ /sbin/stinit -v -v -v -p -f ./tests/data/bad-definition.data

Parsing modes for ('XYZ', 'UVW1', '').
Mode 1 definition:  blocksize=
Warning: errors in definition for ('XYZ', 'UVW1', ''):
 blocksize=

Definition parse completed. Errors found!
```

s390x:

```
$ ./stinit -p -v -v -v -f tests/data/bad-definition.data

Parsing modes for ('XYZ', 'UVW1', '').
Mode 1 definition:  blocksize=
Warning: errors in definition for ('XYZ', 'UVW1', ''):
 �H

Definition parse completed. Errors found!
```

And shelltest fails to parse that binary output, in 4/5 cases, which
by luck caused the builds to fail. From there it was a fun
investigation into trying to find why it behaves like that.

This bug is caught by the recently introduced MSAN build in the GitHub
CI, so will rely less on luck in the future.

I'm not 100% sure on the semantics of the fix, but it should only
affect this case (missing value to argument), so should be a well
localised fix.

Signed-off-by: Iustin Pop <[email protected]>
  • Loading branch information
iustin committed Apr 20, 2023
1 parent 8056c15 commit 34978b4
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions stinit.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,13 @@ static char *find_string(char *s, char *target, char *buf, int buflen)
cp++;
for (cp2 = cp; *cp2 != '"' && *cp2 != '\0'; cp2++)
;
} else
for (cp2 = cp + 1; isalnum(*cp2) || *cp2 == '-'; cp2++)
;
} else {
if (*cp == '\0')
return NULL;
else
for (cp2 = cp + 1; isalnum(*cp2) || *cp2 == '-'; cp2++)
;
}
if (*cp2 == '\0')
return NULL;
have_arg = TRUE;
Expand Down

0 comments on commit 34978b4

Please sign in to comment.