1

The reason I say this, is that recently. Old library code that has been in our project for many years, has recently began generating range checks. In the past, this code built and ran without issue.

Here's an example scenario that's like one that occurred today.

function DoStringStuff(const S: string): string;
var
  i: Integer;
  len: Integer;
begin
...
  while (Result[i] <> #32)
    and (i <= len) do
  begin
    Result[i] := UpperCase(Result[i])[1];
    inc(i);
  end;
...
end;

When i is >len a range check occurs. I fixed it by moving the length check to the beginning. However, I find it strange that it worked at all given the order that's it's evaluating things, where it will ultimately attempt to evaluate an index beyond the string's range.

Any ideas and or suggestions are welcome.

fourwhey
  • 490
  • 4
  • 19
  • Did you change your Delphi version? – Uli Gerhardt May 19 '17 at 19:56
  • What exactly is your question? You know how to fix the code. And we certainly cannot say why behavior changed. There is simply not enough information for us to know. You know what is different and what did you do between the point it "worked" to the point is doesn't. – Dalija Prasnikar May 19 '17 at 20:05
  • And there is compiler option to switch off range check but I don't see the point in doing that in your case. http://stackoverflow.com/questions/4997911/switch-off-delphi-range-checking-for-a-small-portion-of-code-only – Dalija Prasnikar May 19 '17 at 20:12
  • @UliGerhardt No, Delphi 7. I did reinstall on a new machine though, which has Windows 10. – fourwhey May 19 '17 at 20:44
  • @DalijaPrasnikar To avoid having to fix all of the unknown places where this problem might exist but hasn't come up. The code builds, and doesn't become an issue until runtime. – fourwhey May 19 '17 at 20:46
  • Make sure to test your code with all checks on. Make unit tests. What more do you want? – LU RD May 19 '17 at 20:50
  • @LURD It's not possible to create unit tests for legacy code. It would be a full time job and I'm the only developer. I know it sounds like a cop-out but it's the reality of the things. – fourwhey May 19 '17 at 21:00
  • You may want to add {$R-} to units that need it. Used to get this a lot with 3rd Party code that requires {$R-} when my default compile is {$R+}. – FredS May 19 '17 at 21:06
  • There are some tools for investigating code quality, but they can never substitute hands on unit tests. Since your code base is rather old, most errors would have been found by now, but only you can decide what to do. – LU RD May 19 '17 at 21:11
  • 2
    @FredS Er, disabling range checking sounds like a terrible idea. – David Heffernan May 19 '17 at 21:12
  • Why are you doing `UpperCase(Result[i])[1]`? Why not just`UpCase(Result[i])` – MartynA May 19 '17 at 21:49
  • 1
    @MartynA I don't know why the original developer made that choice. I copied and pasted a snipped of the method here. It probably can be refactored, but at the moment I was only interested in fixing the range check. – fourwhey May 20 '17 at 01:53

1 Answers1

2

That code was always broken and if it ever worked then that was purely by chance. There's no compiler switch that will find such a defect at compile time.

In all versions of Delphi, when range checks are enabled, run time errors will be generated when you access beyond the end of the string. If your code code ran without run time errors before then there are but two explanations:

  1. Range checking was previously disabled.
  2. The loop previously broke before the out of range condition.
David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490