1

So, I have the folowing problem. I have 2 PChar variables. I allocate memory for first, do some operations, allocate memory for the second variable - and on this step the first variable contains bad value (i saw it while debugging). Here is the code:

procedure TReadThread.Execute;
Var
  iRead, temp, i, count : Integer;
  header, params : PChar;
begin
  try
    GetMem(header, 12);
    iRead := recv(FSocket, header^, 12, 0);

    if (iRead<>12) then
      raise Exception.Create('Header recieving problem!');

    temp := StrToIntDef(String(copy(header,3,4)),0);

    if (temp=0) then
      raise Exception.Create('Body receiving problem!');

    count := temp*SizeOf(Char);

    if (count+12<=16384) then
      begin
        GetMem(params, count);
        iRead := recv(FSocket, params^, count, 0);

        if (iRead<>count) then
          raise Exception.Create('Cant recieve messsage fully!');
      end
    else
      raise Exception.Create('Bad message size (>16 KB)!');

    GetMem(FText, temp*SizeOf(Char)+12);
    FText := PChar(String(header) + String(params));

    FreeMem(header);
    FreeMem(params);
  except
    on E : Exception do
      ShowMessage(E.Message);
  end;
end;

On the line

iRead := recv(FSocket, params^, count, 0);

When I look for the variable HEADER value - I saw somethind amazing - not the same when I saw at the beginning of the procedure. How I can fix it?

Dmitry Belaventsev
  • 6,347
  • 12
  • 52
  • 75

2 Answers2

3

I presume that FText is a PChar. Since you say that you are using Delphi 2010, you should be aware that Char is actually synonymous with WideChar and is 2 bytes wide. I suspect you really want to be using AnsiChar.

The most glaring problem is that you allocate memory for FText and then discard it with the assignment to FText. What's more, the memory that FText refers to is destroyed when the procedure ends.

I think that you should probably do the following:

  • Switch to AnsiChar for the recv calls.
  • Change FText into AnsiString.
  • Stop using GetMem altogether and use stack allocation.

Perhaps something like this:

procedure TReadThread.Execute;
Var
  iRead, count: Integer;
  header: array [0..12-1] of AnsiChar;
  params: array [0..16384-1] of AnsiChar;
begin
  try
    iRead := recv(FSocket, header, 12, 0);

    if (iRead<>12) then
      raise Exception.Create('Header receiving problem!');

    count := StrToIntDef(Copy(header,3,4),0);

    if (count=0) then
      raise Exception.Create('Body receiving problem!');

    if (count+12<=16384) then
      begin
        iRead := recv(FSocket, params, count, 0);
        if (iRead<>count) then
          raise Exception.Create('Cant receive messsage fully!');
      end
    else
      raise Exception.Create('Bad message size (>16 KB)!');

    SetLength(FText, 12+count);
    Move(header, FText[1], 12);
    Move(params, FText[13], count);
  except
    on E : Exception do
      ShowMessage(E.Message);
  end;
end;
David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • 2
    +1 for not using getmem anymore. Note that you can also use: `var header: ansistring; begin SetLength(header,12);` to create a string with length 12 that is autodestroyed upon exiting the routine. Given the fact that the stack is 1MB, creating a 16KB var looks like a fine solution. – Johan May 21 '11 at 20:51
0

AS David Heffernan Said befor. Char is 2 byte and also pChar point to a Unicode char in Delphi 2010. but the code of David has 2 problems

  1. If you want to get the international chars (unicode or utf8 strings) you can not use the AnsiChar

  2. If define params variable as Array [0..16384-1] of AnsiChar then you will lost your program performance. local variables will use the stack and define the params like as David defined will consume your stack space.

for the answer you can use use your code by 1 simple changes. only define your header and params variable as PAnsiChar. you can put the other codes unchanged.

header, params: PAnsiChar;

Mahdi
  • 247
  • 2
  • 6