Access violation help

[hac3ru]

Code: [Select]

function message_rec.write_msg2(receivedbyte: byte;
  var Count, ID1, OP1, DL1, CHK1: byte;
  var Addr1, Msg1: ArrayOfByte): string;
begin
  Result := 'OK';
  try
    case Count of
      0:
      begin
        ID1 := receivedbyte;
      end;
      1:
      begin
        OP1 := receivedbyte;
      end;
      2..5:
      begin
        Addr1[Count - 2] := receivedbyte;
      end;
      6:
      begin
        DL1 := receivedbyte;
      end;
    end;
    try
      SetLength(msg1, DL1);
    except on E:Exception do
      begin
        result:='SetLength';
      end;
    end;
    if ((DL1 = 0) and (Count >= 7)) then
    begin
    end
    else
    begin
      if ((Count >= 7) and (Count < 7 + DL1)) then
      begin
        Msg1[Count - 7] := receivedbyte;
      end;
    end;
    if (Count = 7 + dl1) then
    begin
      chk1 := receivedbyte;
    end
  except
    on E: Exception do
    begin
      Result := 'Class:' + E.ClassName + sLineBreak +'Message:' + E.Message;
    end;
  end;
end;This is the function I use to form the message I get from an RS232 interface. I take Byte by Byte, increase a counter and based on that counter, I know what the byte is. The problem is that sometimes, random, I get Access Violation. Can anyone tell why?
DL = DataLength. I set the MSG to length of DataLength so everything will get in there.
Sometimes it works fine, sometimes I get access violation.

[Mike.Cornflake]

Hard to say.

One thing stands out, you're only doing minimal error checking.

Code: [Select]

      2..5:
      begin
        Addr1[Count - 2] := receivedbyte;
      end;
This code for instance, contains an implicit assumption that Count is >= 2.  If it's less, then that line will fail.

Always fun finding errors.  I always recommend defensive code.  The above code then becomes

Code: [Select]

      2..5:
      begin
        If (Count>= 2) And Count<Length(Addr1) Then
          Addr1[Count - 2] := receivedbyte
        Else
          // What do you want to do if Count is invalid?
      end;

(Code written off the top of my head, and assumes a) Arrays are 0 based, and b) you get the length of an array by Length() - Modify accordingly if I'm wrong)

[hac3ru]

Since I have a case count 2..5... The count inside the case cannot be less than 2 or greater than 5 right?
I mean.
Forgot to say that Addr1 and msg1 lengths are set in some other function that is always called before this one.

[Mike.Cornflake]

Absolutely correct, I missed that :-)  And I got my bounds check for the upper limit wrong as well.  You can drop the <=2, and change the upper bound check to If (Count-2)<Length(Addr1) Then...   I still recommend the defensive code.  Sure, it may work now, but if you call this method from another place in code, maybe the same assumptions won't hold.

Is the array a fixed length of at least 3 then?   If it's ever less, this will also cause a failure.

Not an AV though.  Should simply be index out bounds...

[hac3ru]

Before I call this function, I set the lengths of both Addr1 and MSG1 to 4... Never less than 3...

[Mike.Cornflake]

Code: [Select]

      if ((Count >= 7) and (Count < 7 + DL1)) then
      begin
        Msg1[Count - 7] := receivedbyte;
      end;

What about this line?  Can you guarantee that DL1 is always < 4?

UPDATE:  Bah!  Ignore.  I see you're SetLength MSG1 inside here.  It's not fixed to length = 4, it's fixed to length = DL1

[Mike.Cornflake]

Can you post the entire unit?  Sorry, I don't have sufficient information here to trace through the code...

[hac3ru]

Ermm... Found the problem. I need to set the length of the Addr to 4 each time I run this function. For some reason, it doesn't stay 4 ...
My logic is that if I set the length of an array outside the function, it would read that array's length inside the function too... Nope... I need to put a

Code: [Select]

Setlength(Addr1, 4) next to the OP=receivedbyte in order to get it working.

Thanks for your time mate.

[taazz]

Ermm... Found the problem. I need to set the length of the Addr to 4 each time I run this function. For some reason, it doesn't stay 4 ...
My logic is that if I set the length of an array outside the function, it would read that array's length inside the function too...

Correct, this is how it should be, setting the length of an array at any point it should retain that length until it is changed again and since you have already declared the addr1 as Var the length and data of the array should be retained between calls if nothing touches them.

Your assumption that you need to call the setlength inside the procedure each time it is called is wrong in general but with out more info on the original array variable especially when it is declared if there are any other statements that will reset the length to an other size etc would be required before making any useful observation/comment.

[hac3ru]

I am initializing the ADDR1 on form create. On create I set the length of Addr1 to 4... The form doesn't get destroyed, the length of addr1 is not changed at any time. Don't know why it is doing that and right now, I don't have the time to look into it. Glad I got it to work....