Reverse String and null

An approach to the Reverse String exercise can loop backwards with something like for (( i = ${#1}; i >= 0; i-- )). This seems to include the null value at the start since Bash strings I think are zero indexed and null terminted.

The use of echo -n seems to mask this. Can/should the tests try and catch this?

Ooh! Nice catch. That loop does have an off by one. But, while bash internally uses null terminated C strings, it doesn’t actually allow you to access the null character or put a null into a string. Instead, you get "", the empty string, when you go beyond the end of the string. Since strings are null terminated, a string cannot start or contain a null. By definition, the string ends at the first null byte. If the first byte “in” a string is a null, then the string is zero characters long and, if you try to use it, you will get an empty string. Does that make sense?

» for (( i = ${#1}; i >= 0; i-- )); do printf '<%s>\n' "${1:i:1}"; done

Would you like to submit a PR to fix the bug?

That all makes sense, thanks @IsaacG ! Is this testable though ? The output appears indistinguishable from the expected output (when using echo -n), so are you imagining the analyzer could do this ?

If it’s not mad bash I’m happy to have go. :D

The bash track doesn’t have any analyzer, that I know of[1]. It has a test runner, which runs the unit tests. The tests simply check the outputs.

I can’t think of any way to test this. The output is identical. Prepending a bunch of empty strings is essentially a no-op so it isn’t something the tests can get at.

There are ways to do static analysis on the code but we don’t currently have any of that set up.

[1] edit: I totally forgot about shellcheck

It does, but it just invokes shellcheck, so it’s more of a linter.

1 Like

@IsaacG, I’m confused: this exercise does not have Approaches? What is a PR for?

@habere-et-dispertire, the tests look at the program’s output, so there’s no way to test how many empty strings were appended to the result. Hopefully this would be noticed in a code review, but it’s a pretty subtle bug that’s easy to miss.

bash arrays do the same thing if there’s no element at the given index: no error, just an empty string.

ary=(aa bb cc dd)
printf '<%s>\n' "${ary[3]}" "${ary[4]}"
# => <dd>
# => <>

My apologies. I’m mostly on mobile this week and read that OP as an issue with an existing Approaches document, and not as an issue with a student solution.

This solution does work. From a test perspective, the output is exactly right. It has an off by one which leads to a no-op loop iteration. This isn’t something tests can find. I assume shellcheck doesn’t flag this.

This is the sort of thing that code reviews can really help with detecting and discussing!