Category Archives: Computing

How not to do exception classes in a Delphi library

Computing, Development, Pontificating

The pngimage unit in Delphi includes a bunch of exception classes to handle error conditions when handling PNG files.  That’s great, except for one problem: all the classes inherit from Exception instead of an abstract EPNGException.  Of course it would be even better to inherit from a hierarchy of abstract exception classes.

Frankly I don’t care if the image is corrupt because it has an unknown critical chunk, or if it is corrupt because it has no image data.  All I care about is that my application cannot load an image and I need that condition handled gently.

Basically, not using a hierarchy of classes means that error handling code in applications loading PNG files must either be too broad and generic (on E:Exception), or too specific (on E:EPNGUnknownCriticalChunk).  Future changes to the PNG class library for additional exception types when loading a PNG file would require every developer to add additional exception checks to every location where PNG files are loaded in order to handle that error, when in nearly every case the developer really just wants to handle EInvalidGraphic!

Even worse, this means that generic code for loading images must be overloaded with knowledge about PNG exception types, or else risk masking more serious errors such as access violations with an overarching on E:Exception clause!

So here’s the current complete list that the developer must be aware of:

EPNGOutMemory = class(Exception);
EPngError = class(Exception);
EPngUnexpectedEnd = class(Exception);
EPngInvalidCRC = class(Exception);
EPngInvalidIHDR = class(Exception);
EPNGMissingMultipleIDAT = class(Exception);
EPNGZLIBError = class(Exception);
EPNGInvalidPalette = class(Exception);
EPNGInvalidFileHeader = class(Exception);
EPNGIHDRNotFirst = class(Exception);
EPNGNotExists = class(Exception);
EPNGSizeExceeds = class(Exception);
EPNGMissingPalette = class(Exception);
EPNGUnknownCriticalChunk = class(Exception);
EPNGUnknownCompression = class(Exception);
EPNGUnknownInterlace = class(Exception);
EPNGNoImageData = class(Exception);
EPNGCouldNotLoadResource = class(Exception);
EPNGCannotChangeTransparent = class(Exception);
EPNGHeaderNotPresent = class(Exception);
EPNGInvalidNewSize = class(Exception);
EPNGInvalidSpec = class(Exception);

I’m disappointed that when this code was acquired and integrated into the Delphi source code, that this level of basic tidy up was not done.

Using out parameters in Delphi SOAP transactions

Bugs, Computing, Development

So here’s today’s little gotcha: TRemotable out parameters in Delphi SOAP interface function calls must always be initialised before the call.  If you don’t do this, at some point your client application will crash with an access violation in TValue.Make (System.Rtti.pas).

It turns out that when the TRIO component parses the Run Time Type Information (RTTI), it conflates the var and out modifiers on the parameters, and treats out parameters as var parameters (see function TRttiMethod.GetInvokeInfo).  This means that when the function is called, the TValue.Make function which collects RTTI for the passed parameters will assume that a non-nil object reference is always pointing to a valid and initialised object, even for an out parameter, and will dereference it to get additional RTTI.

The workaround is simple: just initialise all out parameters to nil before the function call.  This is good practice anyway.

I think the proper fix in the Delphi library code would be to treat out and var parameters differently in the SOAP calls, and always clear out parameters to nil within the SOAP framework.

Here’s some example code to reproduce the problem:

unit TestOutParams;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Vcl.StdCtrls,
  Soap.SoapHTTPClient, Soap.InvokeRegistry;

type
  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
  end;

type
  ITestOutParam = interface(IInvokable)
    ['{343E9171-4300-4523-A926-4904EDD652E1}']
    function TestOutParam(out p5: TSOAPAttachment): Boolean; stdcall;
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

function GetITestOutParam: ITestOutParam;
var
  RIO: THTTPRIO;
begin
  Result := nil;
  RIO := THTTPRIO.Create(nil);
  try
    Result := (RIO as ITestOutParam);
  finally
    if (Result = nil) then
      RIO.Free;
  end;
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  FSOAP: ITestOutParam;
  FOutputParameter: TSOAPAttachment;
begin
  FOutputParameter := Pointer(8); // simulate uninitialized local variable pointing to protected non-nil address
  FSOAP := GetITestOutParam;
  Assert(Assigned(FSOAP));
  FSOAP.TestOutParam(FOutputParameter); // this will crash
end;

initialization
  InvRegistry.RegisterInterface(TypeInfo(ITestOutParam));
end.

What’s up with Adobe updaters?

Computing, Family|Bible|More

This morning, I turned my computer on to be greeted with notification of yet another update to Adobe Reader.

Now I believe that regular software updates are basically a good thing.  Yes, they are a hassle but at least Adobe is fixing bugs and security vulnerabilities, right?

But that’s where the goodness ends.  The update dialog is pretty vague as to what you are getting in the update.  Fortunately there is a Details link, so let’s click on it to see where it takes us.  Right.  It takes us to a list of all the recent releases of Adobe Reader.  So I still have no idea what is changing in this update, or even which version I am being updated to or from. This little release notes page also has an bit of a catch-22 dependency situation whereby you need to have a PDF reader (presumably Adobe Reader) installed in order to read the release notes.  Not the best dependency to have when you are trying to install Reader in the first place.

Clicking the Details link in the update doesn’t give you details of that update. 
It gives you details of all updates from all time

The Adobe Reader updater has one set of issues.  The Adobe Flash Player updater has a different set.  Oh yes, they are separate applications.  And they both run on start up (how often do you restart your computer these days?  Updaters shouldn’t be running on system start up)

The Flash Player updater is reasonably smooth, apart from the confusion of two dialogs appearing in succession, each with a handy check box.  You must not tick the check box in the first dialog, but you must tick the check box in the second dialog that appears.  Got that?  Now tell me, why do we need two dialogs for the install?  I already said “go ahead” by clicking the first Install button.

You don’t want to tick that check box

But you’ve gotta tick this one.  Now click Install again

Just a note about that license agreement link. I think Adobe must have a policy of making it difficult to find information about their products. Clicking that link takes you to a page with EULAs for all 10 thousand of their products, with each EULA in about 5 million different languages. Good luck finding the correct agreement.

But two update programs isn’t enough for one company.  We need more!  So here’s another.  This one also runs at start up and handles a bunch of other Adobe applications.

A plethora of Adobe application updates

How do you think the Adobe Reader update today finished?  Note that the version I got is now visible, now that I’ve installed it.  I give you the dreaded Restart Now button:

WinDBG and Delphi exceptions

Computing, Development

When debugging a Delphi XE2 app in WinDBG, NTSD or a related debugger, it is very helpful to be able to display the actual class name and error message from an exception in the debugger.  The following script will do that for you automatically:

sxe -c "da poi(poi(poi(ebp+1c))-38)+1 L16;du /c 100 poi(poi(ebp+1c)+4)" 0EEDFADE

The command can be broken down as follows:

  • sxe adds a filter for an exception event, enabled by default.  sxd would add the filter, disabled.
  • -c tells the debugger to run the quoted commands when the event occurs.
  • da displays an “ANSI” (non-Unicode) string from memory in the debugger
  • ebp+1c is a pointer to the exception record
  • poi(poi(ebp+1c))-38 points to the non-Unicode string returned by TObject.ClassName, internally known as Self.vmtClassName.
  • du displays a Unicode string from memory in the debugger.
  • poi(ebp+1c)+4 is a pointer to the Unicode string for Exception.Message.  As with all Delphi strings, you could go back 4 bytes to get the string length, but for now we limit the length to 100 characters with the /c parameter. 
  • 0EEDFADE is the exception code for a Delphi exception (another example is C0000005 for Access Violation).

These offsets are correct for Delphi XE2.  For Delphi 7, the relevant command is:

sxe -c "da poi(poi(poi(ebp+1c))-2c)+1 L16;da /c 100 poi(poi(ebp+1c)+4)" 0EEDFADE

Other versions of Delphi will have similar offsets, a little spelunking should yield the appropriate offsets pretty quickly!

Another partial malware diagnosis

Computing, Development, Malware, Tools

A customer reported a problem with starting our application today.  The error reported by our application was strange and was not one we’d encountered before:

Exception 'Exception' in module _________.exe at 004904CB
Unable to hook API functions for print preview [-1,-1,-1,0,0,0,0]

In effect the error told us that 4 out of 7 API hooks failed.  I was called upon to try and diagnose the issue.

Initially I looked for a 3rd party application that could be hooking the calls in question (RegisterClassA, RegisterClassW, RegisterClassExA, RegisterClassExW).  But there were no unusual applications running according to Process Explorer, and no unexpected DLLs in memory in the process.  After disabling the antivirus in case that was causing the problem, and running both RootkitRevealer and Procmon with no clear outcomes, I decided I’d need to go deeper.

Time to break out windbg.  I started our process and looked at the disassembly for one of the functions that failed to hook.  Here’s what I saw:

0:000> u user32!registerclassexw
user32!RegisterClassExW:
7e41af7f 8bff            mov     edi,edi
7e41af81 55              push    ebp
7e41af82 8bec            mov     ebp,esp
7e41af84 8b4508          mov     eax,dword ptr [ebp+8]
7e41af87 833830          cmp     dword ptr [eax],30h
7e41af8a 0f850be70200    jne     user32!RegisterClassExW+0xd (7e44969b)
7e41af90 6800010000      push    100h
7e41af95 6a00            push    0

That’s pretty normal, the usual mov edi,edi that most Windows API calls start with, and what we were expecting.  So I continued execution until the error occurred, and took another look at that point.

0:000> u user32!registerclassexw
user32!RegisterClassExW:
7e41af7f e9cc93d281      jmp     00144350
7e41af84 8b4508          mov     eax,dword ptr [ebp+8]
7e41af87 833830          cmp     dword ptr [eax],30h
7e41af8a 0f850be70200    jne     user32!RegisterClassExW+0xd (7e44969b)
7e41af90 6800010000      push    100h
7e41af95 6a00            push    0
7e41af97 6a00            push    0
7e41af99 50              push    eax

Huh, that’s kinda different.  Now we were jumping off into a very unexpected part of memory.  A quick check of that address revealed that it was not mapped into the normal address space of any modules.  I had a look at the code in question.

0:000> u 144350 L...
00144350 55              push    ebp
00144351 8bec            mov     ebp,esp
00144353 83ec30          sub     esp,30h
00144356 f605e029150004  test    byte ptr ds:[1529E0h],4   ; 001529e0
0014435d 56              push    esi
0014435e 8b7508          mov     esi,dword ptr [ebp+8]
00144361 7433            je      00144396
00144363 e8595bffff      call    00139ec1
00144368 84c0            test    al,al
0014436a 742a            je      00144396
0014436c 85f6            test    esi,esi
0014436e 7426            je      00144396
00144370 833e30          cmp     dword ptr [esi],30h
00144373 7521            jne     00144396
00144375 8b4608          mov     eax,dword ptr [esi+8]
00144378 e8fdfeffff      call    0014427a
0014437d 85c0            test    eax,eax
0014437f 7415            je      00144396
00144381 6a30            push    30h
00144383 56              push    esi
00144384 8d4dd0          lea     ecx,[ebp-30h]
00144387 51              push    ecx
00144388 e818010000      call    001444a5
0014438d 8945d8          mov     dword ptr [ebp-28h],eax
00144390 8d45d0          lea     eax,[ebp-30h]
00144393 50              push    eax
00144394 eb01            jmp     00144397
00144396 56              push    esi
00144397 ff1568141300    call    dword ptr ds:[131468h]   ; -> 02200126
0014439d 5e              pop     esi
0014439e c9              leave
0014439f c20400          ret     4

A bit hard to know what it was doing but there was a call at the bottom there that was worth a quick look.

02200126 8bff            mov     edi,edi
02200128 55              push    ebp
02200129 8bec            mov     ebp,esp
0220012b e954ae217c      jmp     user32!RegisterClassExW+0x5 (7e41af84)

Yep, as expected it was a jump back to the original API function, 5 bytes in. That looked like a hook library was being used because the callback to the original function was in a separate memory block.  But no real info.  But again, looking at the address space revealed it belonged to no known module.

0:000> !address 2200126
    02200000 : 02200000 - 00001000
                    Type     00020000 MEM_PRIVATE
                    Protect  00000040 PAGE_EXECUTE_READWRITE
                    State    00001000 MEM_COMMIT
                    Usage    RegionUsageIsVAD

At this stage, it was clear we were looking at malware, so I decided to look for some strings in the data area referenced earlier (in blue, above).  Initially I found only strings pointing to Application Data and other uninteresting sources.

0:000> dd 1529e0
001529e0  00000000 02181ea0 0000001c 83f6f0a1
001529f0  00000000 00130000 7c800000 7c900000
00152a00  02200000 00000000 7c90d7fe 00000000
00152a10  0220000a 7c916a02 0000000c 00152a24
00152a20  00000000 00040001 00000000 00000000
00152a30  00000000 00000000 ffffffff 02181ee0
00152a40  003a0043 0044005c 0063006f 006d0075
00152a50  006e0065 00730074 00610020 0064006e

But eventually I struck gold:

0:000> dd
00152f5c  00000000 000006b0 00000000 004f0053
00152f6c  00540046 00410057 00450052 004d005c
00152f7c  00630069 006f0072 006f0073 00740066
00152f8c  0041005c 006b0067 00610065 00000064
00152f9c  00000000 00000000 00000000 00000000
00152fac  00000000 00000000 00000000 00000000
00152fbc  00000000 00000000 00000000 00000000
00152fcc  0019c110 ffffffff 00000000 00000000

This proved to be a suspicious registry key:

0:000> du 152f68
00152f68  "SOFTWARE\Microsoft\Agkead"

A quick glance at that registry key showed the following suspicious registry entries:

I picked up a few other interesting strings as well:

0:000> du 152fe8
00152fe8  "Global\{451EEC04-7C31-7A30-8C56-"
00153028  "BCE6C174342E}"
0:000> du 1527e0
001527e0  "Enfok"

The following string was also interesting:

0:000> du 1523d4
001523d4  "\Documents and Settings\Receptio"
00152414  "n_2.PGE\Application Data\Ewacg\o"
00152454  "xmo.hio"

While the folder existed, I was unable to see the file oxmo.hio.  This, as well as the fact that I could not see any user mode activity doing the hooking of the functions in question, really suggested a rootkit which was doing some cloaking, rather than simple user-mode malware.

A reference to the string Agkead was on ThreatExpert.

But by now I was really only continuing out of interest, so I handed the machine in question back to the client, with the advice that they rebuild it — difficult to be sure that the machine is clean any other way.  While it would have been fun to analyse the malware further, it’s not really my job 🙁

A story, or will pay anybody five pounds to remove database from one computer to another

Computing, Networking, SQL, Tools

Today I needed to download a recent backup of a database from a server in the data centre for testing major changes to the database locally…  Things don’t really go as well as I expect:

  1. Backup is 12GB.  Yike, that’s going to take ages to download!
  2. OK, so let’s zip it.  20 minutes later… we have a 2GB zip file.  Well, that was worth doing.
  3. Now copy (encrypted) zip to a server with HTTP access (much faster than downloading over SSH).  About 1 minute (yep, nice fast network in the data centre)
  4. Download zip to database server on local network.  Takes about 45 minutes.
  5. So try to unzip on server with Win Server 2003 Zip tool.  Huh.  It doesn’t support files > 4GB (plus it’s running low on disk space, so a bit of a juggling act to have enough space to unzip anyway).
  6. OK, move the zip file to a client machine.  5 minutes on LAN
  7. Unzip locally then move 12GB backup back to server.  Best part of an hour.  Yes, it’s a slow client machine and the LAN is only 100mbit.
  8. Try to restore to SQL Server.  Oops, old version of SQL Server on that server.
  9. So install SQL Server 2008 on another server.  That takes 45 minutes (including adding .NET framework).
  10. Can’t do anything with it until we have patched SQL Server.  So download and run SQL Server 2008 SP3.  20 minutes to download.
  11. Installer starts while I’m not at computer, automatically cancels when screensaver starts (consent.exe)
  12. Drat.  Download SQL Server 2008 SP3 again as IE has deleted installer from cache.  This time I save the service pack installer.  20 minutes.
  13. Finally! Install SQL Server 2008 SP3.  10 minutes.
  14. Restore database to new server.  5 minutes.

And finally it works.

Why do the little things take so much energy?

Working around limitations in Strava’s Segment Efforts API

Computing, Cycling, Strava

The Strava API has a call to retrieve the first 50 efforts for a segment.  In theory, the API supports an offset parameter to allow you to download additional efforts after the first 50.  However the offset parameter does not work currently in the segment efforts API.  I didn’t want to wait until V3 of the API, so…

Additionally, the segment efforts API returns efforts ordered fastest to slowest, so a naive date-based retrieval does not work, and this may also be why the offset parameter is not currently supported.

I now have a workaround which does the job (albeit a little more slowly): use the startDate and endDate parameters, which do work, to initially pull the efforts across “all time” (or, say, from 2010 to now).  If you receive 50 efforts back from the call, then split the time window in half, and retrieve each half.  Rinse and repeat until you receive less than 50 efforts for a window, at which point you know you have all the efforts for that window.  It’s simple enough to merge the arrays that you receive in response.

This works fine, but is a bit more load on the Strava servers.  For example, a segment with 425 efforts required 28 calls as opposed to 9 if the offset parameter worked, or just 1 if you could request the full set of efforts (which is still not a huge download, although really busy segments may be a bit more problematic).

I now optimise to retrieve all efforts for segments just once, then just the last few days worth later on.  The downside is that new subscribers will not have old efforts uploaded (to resolve this I may periodically do the full sync again). New segments would also bump into this issue until a global sync is done.

My implementation is not perfect (I’m ignoring errors for now), and if there are 50 efforts in less than 1 second then I’ll get none of them 🙂  Nevertheless, this little PHP code snippet is one way it could be done.

  function getSegmentEfforts($segmentId, $startDate = 0, $endDate = 0)
  {
    if($startDate == 0 && $endDate == 0)
    {
      $startDate = mktime(0,0,0,1,1,2010);
      $endDate = time();
    }
   
    if($startDate >= $endDate) return null;
   
    $startDateString = date(“Y-m-d\\TH:i:s\\Z”, $startDate);
    $endDateString = date(“Y-m-d\\TH:i:s\\Z”, $endDate);

    $efforts = $this->callApi(“/v1/segments/$segmentId/efforts?startDate=$startDateString&endDate=$endDateString”);
    $numEfforts = sizeof($efforts->efforts);
   
    if($numEfforts == 50)
    {
      // split the time period in 2
      $midDate = round(($startDate + $endDate) / 2, 0);
      $firstEfforts = $this->getSegmentEfforts($segmentId, $startDate, $midDate);
      $secondEfforts = $this->getSegmentEfforts($segmentId, $midDate, $endDate);
      if($firstEfforts == null) return $secondEfforts;
      if($secondEfforts == null) return $firstEfforts;
      $firstEfforts->efforts = array_merge($firstEfforts->efforts, $secondEfforts->efforts);
      return $firstEfforts;
    }
   
    return $efforts;
  }

Weird filename globbing in Windows

Bugs, Computing, Windows

Can anyone explain this result?  (note: folder name has been obscured)

D:\...\exe\tds>dir *337*
 Volume in drive D has no label.
Volume Serial Number is 4279-DECE

Directory of D:\...\exe\tds

05/12/2011  02:39 PM        12,432,384 tds-8.0.324.0.exe
11/04/2011  02:41 PM        13,268,480 tds-8.0.337.0.exe
2 File(s)     25,700,864 bytes
0 Dir(s)  129,676,939,264 bytes free
D:\...\exe\tds>dir *337.0.exe
Volume in drive D has no label.
Volume Serial Number is 4279-DECE

Directory of D:\...\exe\tds

11/04/2011  02:41 PM        13,268,480 tds-8.0.337.0.exe
1 File(s)     13,268,480 bytes
0 Dir(s)  129,676,324,864 bytes free

I can’t figure out why tds-8.0.324.0.exe is matching *337*, in this folder.  There are lots of other files in the folder with similar names that don’t match.  chkdsk returned no errors.  Any ideas?

Update 4:45pm: I did a little more investigation.  The error can be reproduced on other computers, so it is not related to the state of the filesystem, or the path name.  The following command narrowed down the match a little:

c:\temp\tds>dir *d337*
Volume in drive C is OS
Volume Serial Number is 8036-7CEB

Directory of c:\temp\tds

12/05/2011  02:39 PM        12,432,384 tds-8.0.324.0.exe
1 File(s)     12,432,384 bytes
0 Dir(s)  12,700,200,960 bytes free

But the following did not match:

c:\temp\tds>dir td337*
Volume in drive C is OS
Volume Serial Number is 8036-7CEB

Directory of c:\temp\tds

File Not Found

Curiouser and curiouser.  Procmon did not provide much insight into the issue:

procmon

Where to now?  It’s Sunday afternoon, and I don’t feel like breaking out a kernel debugger to trace that any further right now.

Update 7:30pm: Well, I was puzzled so I did a little more research.  And ran across a mention of 8.3 filenames.  All of a sudden everything clicked into place.

D:\...\exe\tds>dir /x *337*
Volume in drive D has no label.
Volume Serial Number is 4279-DECE

Directory of D:\...\exe\tds

05/12/2011  02:39 PM        12,432,384 TDD337~1.EXE tds-8.0.324.0.exe
11/04/2011  02:41 PM        13,268,480 TDD938~1.EXE tds-8.0.337.0.exe
2 File(s)     25,700,864 bytes
0 Dir(s)  129,670,885,376 bytes free

Yes, even today DOS comes back to bite us.  So just beware when doing wildcard matches — especially with that old favourite del.

Workaround for the "AllowDocumentFunction constraint violated" error with Delphi XE2 apps

Computing, Development

I have been reading discussions online about an EOleException error we were getting when calling TransformNode from a Delphi XE2 application: "Operation Aborted: AllowDocumentFunction constraint violated". The problem arises because Delphi XE2 now uses MSXML 6.0, which has made some changes to default settings for security reasons.

This, along with the ProhibitDTD property, has caused some grief. The recommended fix is to make changes to the VCL unit xml.win.msxmldom.pas. I found an alternative which appears to work without side-effects and requires no changes to the VCL source code: set the MSXMLDOMDocumentCreate to your own custom implementation which sets the AllowDocumentFunction (and ProhibitDTD or AllowXsltScript if you wanted) property after creating the object.

unit msxml_transformnode_fix;

interface

implementation

uses
  Winapi.ActiveX, Winapi.Windows, System.Variants, System.Win.ComObj, Winapi.msxml, Xml.xmldom, System.Classes,
  Xml.Win.msxmldom, Xml.XMLConst;

function TryObjectCreate(const GuidList: array of TGuid): IUnknown;
var
  I: Integer;
  Status: HResult;
begin
  Status := S_OK;
  for I := Low(GuidList) to High(GuidList) do
  begin
    Status := CoCreateInstance(GuidList[I], nil, CLSCTX_INPROC_SERVER or
      CLSCTX_LOCAL_SERVER, IDispatch, Result);
    if Status = S_OK then Exit;
  end;
  OleCheck(Status);
end;

function CreateDOMDocument: IXMLDOMDocument;
begin
  Result := TryObjectCreate([CLASS_DOMDocument60, CLASS_DOMDocument40, CLASS_DOMDocument30,
    CLASS_DOMDocument26, Winapi.msxml.CLASS_DOMDocument]) as IXMLDOMDocument;
  if not Assigned(Result) then
    raise DOMException.Create(SMSDOMNotInstalled);

  try
    (Result as IXMLDOMDocument2).SetProperty('AllowDocumentFunction', True);
  except on E: EOleError do
    ;
  end;
end;

initialization
  MSXMLDOMDocumentCreate := msxml_transformnode_fix.CreateDOMDocument;
end.

Delphi’s ongoing problem with "with"

Bugs, Computing, Development

Delphi has long included a scoping construct called with which can be used to increase the readability and efficiency of code.  From Delphi’s documentation:

A with statement is a shorthand for referencing the fields of a record or the fields, properties, and methods of an object. The syntax of a with statement is:
  with obj do statement
or:
  with obj1, …, objn do statement
where obj is an expression yielding a reference to a record, object instance, class instance, interface or class type (metaclass) instance, and statement is any simple or structured statement. Within the statement, you can refer to fields, properties, and methods of obj using their identifiers alone, that is, without qualifiers.

However, with has a really big gotcha: scope ambiguity.  Scope ambiguity came back to bite us yet again today: I am currently upgrading a major project (over 1 million SLoC) from an older version of Delphi to Delphi XE2. One component of this project is a legacy third party component package which is no longer supported.  We have full source and the cost of replacing it would be too high, so we are patching the source where needed.

While tracing a reported window sizing bug, I found the following line of code (on line 12880, yes it’s a big source file):

with vprGetTranspBorderSize(BorderStyle) do
  R := Rect(Left,Top,Width-Right,Height-Bottom);

vprGetTranspBorderSize returns a TRect.  In the earlier version of Delphi, the Width and Height properties were not members of TRect, so were found in the parent scope, being the component itself in this case.  All good.

But now in Delphi XE2, records can now have functions and properties just like a class.  So TRect has a bunch of additional properties, including Width and Height.  Handy to have, until you throw in some code that was written before these new properties existed.  One fix here is simply to qualify the scope:

with vprGetTranspBorderSize(BorderStyle) do
  R := Rect(Left,Top,Self.Width-Right,Self.Height-Bottom);

Or, you could declare a second variable, and eliminate the with statement entirely:

R2 := vprGetTranspBorderSize(BorderStyle);
R := Rect(R2.Left,R2.Top,Width-R2.Right,Height-R2.Bottom);

The problem with the second approach is it makes the code less readable and the introduction of the second variable R2 widens its scope to the whole function, rather than just the block where its needed.  This tends to lead to reuse of variables, which is a frequent source of bugs.

Many developers (including myself) have argued that adding support for aliases to with would avoid these scope problems.  Some others argue that one simply shouldn’t use with, and instead declare variables.  But with does make code much cleaner and easier to read, especially when used with try/finally blocks.

with R2 := vprGetTranspBorderSize(BorderStyle) do
  R := Rect(R2.Left,R2.Top,Width-R2.Right,Height-R2.Bottom);

Given that this issue was reported to Borland as far back as 2002, and since that time many, many complex language constructs have been added to Delphi, I think it’s a real crying shame that Borland/Inprise/CodeGear/Embarcadero have chosen to ignore this problem for so long.