How should I adapt my code for compatibility between TBytes and TIdBytes?

Question

I am having the same problem as mentioned in "Delphi XE4 Indy compatibility issue between TBytes and TidBytes ", i.e. compatibility issues between TBytes(Delphi RTL) and TIdBytes(Indy) datatypes when compiling with the Delphi XE4. The source of my problem is that the code is not exactly according to Indy's interface and some of functions use TBytes, instead of TIdBytes, when calling native Indy IO procedures.

So I was wondering what will the best fix be?

As I see it there are two approaches:

Refactor all functions in the project to use TIdBytes rather than TBytes.
Implement a TBytesToTidBytes conversion procedure (converts the TBytes to TIdBytes) and call that procedure prior to making the mentioned native Indy calls.

Which of the approaches is better/best? Do you have any other ideas on how I can do that?

FYI: The project I am trying to configure with the XE4 is available online on sourceforge : http://sourceforge.net/projects/indy10clieservr/?source=directory

The suggested conversion procedure should be something like:

procedure TBytesToTIdBytes(const Input:TBytes, var Output: TIdBytes)
var 
    i,L : Integer;
    allocate : Boolean;
begin
    L := Length(Input);
    if(Length(Output) <> L) then 
    begin 
        SetLength(Output,L);
    end;
    if(L > 0) then 
        move(Pointer(Input)^,Pointer(Output)^,L);
end;

Your `allocate` variable serves no purpose, but your length check is dangerous. If the arrays have the same length, then you won't reallocate the array, but if the target array has a reference count greater than one, you'll end up overwriting an array you might not intend to. Code holding the other reference to the array might continue expecting the original data. It's safer to reallocate unconditionally. — Rob Kennedy, Sep 17 '13 at 13:34
I see your point. However why do you think the allocate variable is useless? It is intended to indicate whether or not the target array has been previously allocated. — kenny, Sep 17 '13 at 14:24
It's useless because the only time it really affects whether you call `SetLength`, that call to `SetLength` has no net effect. The only time the value of `allocate` makes you call `SetLength` is when it's true. The only way `allocate` is true is when `Output` is null, which means it's an empty array. If `Input` is an empty array, then you didn't need to call `SetLength` because it will just make `Output` empty, which we know it already was. If `Input` isn't empty, then the length comparison would have evaluated to true *anyway*, even without checking the value of `allocate`. — Rob Kennedy, Sep 17 '13 at 15:44
Duly Noted! However, since I am working with large amounts of data, I guess the simpler typecasting solution is preferable. Thanks. — kenny, Sep 18 '13 at 08:44
As an alternative to battling with incompatible types, you could declare your functions to receive data in open arrays. — David Heffernan, Sep 18 '13 at 12:03

Remy Lebeau · Accepted Answer · 2013-09-17T17:46:14.247

7

TBytes and TIdBytes are both implemented as dynamic arrays, they are simply declared differently. The "politically correct" solution is to make a copy of the bytes. But that can waste memory for large arrays. A simpler solution is to use a typecast so you can utilize the array's internal reference count, eg:

type
  PIdBytes = ^TIdBytes;
var
  B1: TBytes;
  B2: TIdBytes;
begin
  B1 := ...;
  B2 := PIdBytes(@B1)^;
end;

Or simply:

var
  B1: TBytes;
  B2: TIdBytes;
begin
  B1 := ...;
  B2 := TIdBytes(B1);
end;

edited Sep 17 '13 at 17:46

answered Sep 17 '13 at 15:51

Remy Lebeau

555,201
31
458
770

Why not simply `B2 := TIdBytes(B1)`? – David Heffernan Sep 17 '13 at 15:52
That works to. I have a C++ background, so I'm more comfortable using pointers in typecasts. – Remy Lebeau Sep 17 '13 at 17:45

score 1 · Answer 2 · edited Sep 17 '13 at 16:22

1

Both types are not the same at implementation level, in newer Delphi versions (TBytes is a simple alias for TArray<Byte> in recent Delphi releases).

So I guess you can use such a procedure:

procedure TBytesToTIdBytes(const Input: TBytes; var Output: TIdBytes);
var L: integer;
begin
  L := Length(Input);
  SetLength(Output,L);
  move(Input[0],Output[0],L);
end;

Here move() is faster than a loop.

edited Sep 17 '13 at 16:22

David Heffernan

601,492
42
1,072
1,490

answered Sep 17 '13 at 15:37

Arnaud Bouchez

42,305
3
71
159

Be sure to check for `L > 0` before calling `Move()` so you avoid an out of bounds error. – Remy Lebeau Sep 17 '13 at 15:43
Or use `move(Pointer(Intput)^,Pointer(Output)^,L)`. – David Heffernan Sep 17 '13 at 15:51
@RemyLebeau OOB error will only happen if range checking is enabled. Code above will just work with no problem with default compiler settings. – Arnaud Bouchez Sep 18 '13 at 18:16
@ArnaudBouchez: better to be safer now than sorry later. Defensive coding is your friend. – Remy Lebeau Sep 18 '13 at 20:16

How should I adapt my code for compatibility between TBytes and TIdBytes?

2 Answers2

Linked