views:

531

answers:

3

Hi, I have a few const arrays of the same base type but different sizes, and I need to point to them in the const records. The code below compiles successfully, but finishes with error.

type
  Toffsets = array of integer;

  Trec = record
             point1: Tpoint;          //complete size
             point2: Tpoint;
             aOffsets:  ^Toffsets;
           end;

const
  cOffsetsA: array [0..3] of integer = (7, 4, 2, 9);
  cOffsetsB: array [0..5] of integer = (1, 2, 3, 4, 5, 6);

  cRec1: Trec = (
    point1:   (x: 140; y: 46);
    point2:   (x: 5; y: 7);
    aOffsets: @cOffsetsA;
  );

  cRec2: Trec = (
    point1:   (x: 40; y: 6);
    point2:   (x: 5; y: 7);
    aOffsets: @cOffsetsB;
  );

In my code I need to access data from the cOffsetsA/B arrays having a pointer to the record. I tried to do it like this:

var pMyRec: ^Trec;
...
pMyRec := @cRec1;
...
i := pMyRec^.aOffsets^[0];

and this causes error - 'Access violation ... read of address 000000...'

Can anybody explain is wrong here and how to fix it, how should it be done?

Probably I will also have to add the _length field in the record, which will hold the size of the array the pointer is pointing to; this is not a problem.

Best regards, LUK

A: 

It looks like I found the answer myself:

type
  Toffsets = array of integer;

is a dynamic array type and setLength must be used with any variable of this type to allocate the memory. What I need is a pointer to an existing constant array, so the only thing which must be changed in order to fix the problem is the declaration of this type. It should look like:

type
  Toffsets = array [0..0] of integer;

and yes, I have to add the _length field in the records as low(pMyRec^.aOffsets^), high(...) and length(...) do not work.

Best regards, LUK

LUK
Actually [0..0] is a bad idea, because it forces you to turn off range checking.Making the upper bound something like "(MaxInt div SizeOf(Integer))-1" should work much better.
Jeroen Pluimers
Have a look at the code for TList etc - it does something similar. TPointerList = array[0..MaxListSize - 1] of Pointer;MaxListSize = Maxint div 16
Gerry
Thank you, I have updated my code accordingly.
LUK
A: 
PhiS
he he,you are right, I have just checked it.Thanks.
LUK
No, it shouldn't work. You're type-casting a pointer-to-TOffsets to tell the compiler that it's really an TOffsets. In fact, the thing stored there is **neither**. It's a pointer-to-array-of-four-integers. TOffsets is a dynamic array, which is not the same as a non-dynamic array.
Rob Kennedy
@Rob - I'm kind of inclined to agree with you, that maybe it shouldn't work. But it does work.
PhiS
The error in the field declaration combined with the error in the field initialization actually cancel each other out, making your erroneous type-cast *appear* to work. According to the declaration, aOffsets points to a dynamic array, but the initialization makes it point to the first element of a static array. The type-cast tells the compiler it points to the first element of a dynamic array. The code will fail when range checking is enabled because static arrays have no length field. Why does anyone ever turn off range checking? Wrong code that accidentally works is still wrong code.
Rob Kennedy
Let's face it these records should be objects! If I was going where He is I wouldn't start from here come to mind :)
Despatcher
I agree that it is mixes static and dynamic arrays. It is not the way I have done it.RE range checking - In Delphi (delphi 6 at least) range checking is disabled by default. Quote from HELP: "Enabling range checking slows down your program and makes it somewhat larger, so use the {$R+} only for debugging.".
LUK
+1  A: 

While the use of constants like this are okay in Delphi, I just want to add that it's in general not a very good idea to do things this way. Personally, if this code is in some unit, I would use something like this: (D2007 and higher)

type
  Toffsets = array of integer;
  Trec = record
    point1: Tpoint; //complete size
    point2: Tpoint;
    aOffsets: ^Toffsets;
    constructor Create(X1, Y1, X2, Y2: Integer; Offsets: array of integer);
  end;

Thus, the record gets a constructor which will fill it with the proper data. As a disadvantage, you can't declare it as a const anymore, if you want to use the constructor to fill it with data. However, you can solve this by using the following code in the Initialization section:

var
  cRec1: Trec;

initialization
  cRec1 := Trec.Create(140, 6, 5, 7, cOffsetsA);

This code will declare the record as a global variable instead of constant, and fill it with your data.

Your constructor could look like this:

constructor Trec.Create(X1, Y1, X2, Y2: Integer; Offsets: array of integer);
var
  I: Integer;
begin
  point1.X := X1;
  point1.Y := Y1;
  point2.X := X2;
  point2.Y := Y2;
  new(aOffsets);
  SetLength(aOffsets^, 0);
  for I in Offsets do
  begin
    SetLength(aOffsets^, Succ(Length(aOffsets^)));
    aOffsets^[Pred(Length(aOffsets^))] := I;
  end;
end;

Which will fill the record with your data. However, you don't have to use the constructor to create records this way! It's just an added function.

To make it even more interesting, you can add properties for the two points and array, making the fields themselves private and only declaring read method for the properties. This way, you can still make sure it's content is read-only and stays read-only.

But what's the risk of using your code? Well, since you declare them all as constants, there isn't much risk, unless you allow assignable typed constants. ({$J+} is declared...) In that case, a piece of code could change a value in cOffsetsA and it would also change in cRec1. Is that what you want?

Anyway, by using a constructor and by initializing them in code, you make your code more dynamic. However, it still continues to stay a simple record type and the additional properties and methods won't change it's structure or size.

You will need Delphi 2007 or better for this, though...

Workshop Alex
Thanks for your reply.It is different approach. I am going to leave the code as it is, mainly because it works. My small comments are:1. I use Delphi 6 :)2. In my real code there is much more data fields in the records so passing them all to the constructor would not be a good idea.3. I do not like having two copies of the same data (one in memory, and one in the code), but this is probably because I write a lot of embedded software, with limited resources.Regards,LUK
LUK
Good points. :-) It's still useful information for others, though. You appear to have a valid reason to use this method, so it's fine.
Workshop Alex
I strongly question whether `aOffsets` really needs to be a *pointer* to a dynamic array. I suspect it was only declared that way to allow the (wrong) code in the question to compile and that an ordinary dynamic array would be sufficient.
Rob Kennedy
Rather than extend and copy one array element at a time, duplicate the entire array in one statement: `aOffsets^ := Copy(Offsets);`
Rob Kennedy
TGwo good points. I made aOffset a pointer to keep it similar to the Q code. Copying the array elements like I did can also be optimized in dozens of ways, but my example can also be used when structures are completely different. By default, I write my code like the example. When the code works, I start thinking about optimizations. The loop is only run once during start-up so optimizing it will gain about 1 milli-second, if you're lucky, on the whole process. Not a big gain...
Workshop Alex