The Wiert Corner – irregular stream of stuff

Jeroen W. Pluimers on .NET, C#, Delphi, databases, and personal interests

  • My badges

  • Twitter Updates

  • My Flickr Stream

  • Pages

  • All categories

  • Enter your email address to subscribe to this blog and receive notifications of new posts by email.

    Join 1,641 other followers

Why SizeOf for character arrays is evil: stackoverflow question “Why does this code fail in D2010, but not D7?”

Posted by jpluimers on 2010/05/11

This Why does this code fail in D2010, but not D7 question on stackoverflow once again shows that SizeOf on character arrays usualy is evil.

My point in this posting is that you should always try to write code that is semantically correct.

By writing semantically correct code, you have a much better chance of surviving a major change like a Unicode port.

The code below is semantically wrong: it worked in Delphi 7 by accident, not by design:
Like many Windows API functions, GetTempPath expects the first parameter (called nBufferLength) number of characters, not the number of bytes.

In Delphi 2009 and up, the code trashes the stack.
The compiler does not warn you, because syntactically the code is right.

But since the code is semantically wrong, you get very strange run-time behaviour: the stack gets trashed, so all sorts of havoc can take place.
The effect is similar to a buffer overflow, and it is exactly this kind of code that hackers use to get access to things they should not get access to.

Changing SizeOf into Length solves the problem.

In Delphi 2009 and up, SizeOf a character array is twice the Length because of Unicode support.
Up until Delphi 2007, the code was still semantically wrong, but you were saved by the fact that a character was always one byte.

function GetTempPathAndFileName(const Extension: string):  string;
var
  Buffer: array[0..MAX_PATH] of Char;
begin
  repeat
    GetTempPath(SizeOf(Buffer) - 1, Buffer);
    GetTempFileName(Buffer, '~', 0, Buffer);
    Result := Buffer;    // <--- crashes on this line
    Result := ChangeFileExt(Result, Extension);
  until not FileExists(Result);
end; { GetTempPathAndFileName }

When porting code, always be aware that the compiler just checks if things are syntactically OK.
You are responsible for making it semantically correct.
If you do that the first time, it saves you a lot of hassle.
Average software developers spend 80% of their time is doing maintenance, so investing in getting the 20% right, saves you a lot of time in the end.

(sometimes I wish, I hadn’t learned those lessons the hard way <g>)

–jeroen

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

 
%d bloggers like this: