Why does calling GetMem lead to a memory leak?

Question

i have this code to see if my foreground window is SDImainframe.

Function Active_window_mf() :Boolean;
var
  FromClass: PChar;
begin

  MFhandle := GetForeGroundWindow;
  GetMem(FromClass, 100);
  GetClassName(MFhandle, PChar(FromClass), 800);
  if StrPas(FromClass) = 'SDIMainFrame' then
    result := true;
end;

However, MADExcept reports that Getmem function has issues. Could anyone suggest, what is wrong in my code?

This question amounts to asking why allocating memory leads to a leak. — David Heffernan, Mar 11 '13 at 16:12
Surely `MFhandle` should be a local variable, it you really need to use a variable at all... — Andreas Rejbrand, Mar 11 '13 at 16:17
I would do it [`this way`](http://pastebin.com/G9M6JHHF). @Andreas, increase your `CN` to be 256 + NULL terminator long. Class name can be at most 256 chars long, but [`GetClassName`](http://msdn.microsoft.com/en-us/library/windows/desktop/ms633582(v=vs.85).aspx) function requires a buffer which can include also NULL terminator. — TLama, Mar 11 '13 at 16:20
I am not sure the question is *that* bad that it need to be closed. After all, beginners might actually believe that the memory allocated by `GetMem` is freed automatically. — Andreas Rejbrand, Mar 11 '13 at 16:20
Thanks, @TLama, I wasn't sure if you included the NUL in that restriction. But shouldn't `nMaxCount` be 257, then? The documentation says `The length of the lpClassName buffer, in characters.` Maybe 'characters' means 'non-NUL characters'. And of course you should check for errors. — Andreas Rejbrand, Mar 11 '13 at 16:21
@TLama: I see from your (now deleted) post that you believe `257` to be the appropriate value? — Andreas Rejbrand, Mar 11 '13 at 16:27
@Andreas, yes. 256 for maximum length of a class name and 1 for NULL terminator. I seriously don't get why this question is going to be closed... — TLama, Mar 11 '13 at 16:29
@TLama: I also was somewhat annoyed by [this](http://stackoverflow.com/questions/15312874/convert-image-to-matrix) — Andreas Rejbrand, Mar 11 '13 at 16:38
@jimsweb Don't mix calling a Windows API function and checking whether the class name equals `'SDIMainFrame'`. Write a function that receives an `HWND` parameter and returns a `string` containing the class name. You can write that function exactly once. From then on in you can call it and get back a nicely wrapped up Delphi `string`. It's really important to code at the right abstraction level. — David Heffernan, Mar 11 '13 at 16:41

Andreas Rejbrand · Accepted Answer · 2016-05-23T07:17:39.040

There are three problems in your code. First, if you allocate memory (GetMem), you need to free it (FreeMem):

GetMem(p, 1024);
try
  // Do sth with the memory
finally
  FreeMem(p);
end;

Second, I don't see the relation between 100 and 800. In fact, you lie. You allocate a buffer of 100 bytes, and then you tell Windows that it is big enough to hold 800 Unicode characters.

Third, the return value of the function is undefined, unless the comparison is true. Hence, you need either to add result := false to the beginning of the procedure, or replace the two last lines (before end;) by

result := string(FromClass) = 'SDIMainFrame'

Anyhow, it's better not to use GetMem at all. I'd do it like this:

var
  CN: array[0..256] of char;
begin
  GetClassName(MFhandle, CN, 256)

Also, you should check for errors. If GetClassName returns 0, an error occurred. Hence, you could do something like

function Active_window_mf(): boolean;
var
  CN: array[0..256] of char;
begin
  result := false;
  if GetClassName(GetForegroundWindow, CN, 257) > 0 then
    result := string(CN) = 'SDIMainFrame';
end;

Update: According to David's excellent point about abstraction levels, it would be nice to do it like this:

function ClassNameFromHWND(const Handle: HWND): string;
var
  CN: array[0..256] of char;
begin
  result := '';
  if GetClassName(Handle, CN, 257) > 0 then
    result := CN;
end;

function Active_window_mf(): boolean;
begin
  result := ClassNameFromHWND(GetForegroundWindow) = 'SDIMainForm';
end;

Why does calling GetMem lead to a memory leak?

1 Answers1