Should I close a handle with IcmpCloseHandle?

Question

I try to write a ping thread searching for connected devices to a local network. I need to avoid indy because it works on an administration account in windows.

I translated c++ example of msdn. Should I use IcmpCloseHandle? If so why should I close the handle which is just a cardinal variable. If not so what's the method for?

function IcmpSendEcho(ICMPHandle: Cardinal; DestinationAddress: Integer; RequestData: Pointer; RequestSize: Word; RequestOptions: Pointer; ReplyBuffer: Pointer; ReplySize: Cardinal; TimeOut: Cardinal): Cardinal; stdcall; external 'icmp.dll';

function TForm1.Ping(IPAddress: string; TimeOut: Integer): Integer;
var
  ICMPHandle: Cardinal;
  DestinationAddress: Integer;
  ReplyBuffer: pICMPEchoReply;
  RequestData: array[0..31] of AnsiChar;
  ReplySize: Cardinal;
begin
  ICMPHandle := IcmpCreateFile;

  DestinationAddress := inetaddr(pansichar(AnsiString(IPAddress)));

  RequestData := 'data buffer';

  ReplySize := SizeOf(ticmpechoreply) + SizeOf(RequestData);

  ReplyBuffer := AllocMem(ReplySize);

  IcmpSendEcho(ICMPHandle, DestinationAddress, @RequestData, SizeOf(requestdata), nil, ReplyBuffer, ReplySize, TimeOut);

  Result := replybuffer.Status;

  FreeMem(ReplyBuffer);
end;

David Heffernan · Accepted Answer · 2016-03-01T18:38:28.503

The documentation for IcmpCreateFile is rather weak. It should make the point that an handle successfully returned by IcmpCreateFile should be closed with a call to IcmpCloseHandle when it is no longer needed. So yes, you do need a call to IcmpCloseHandle.

You are mistaken in thinking that IcmpCreateFile returns a Cardinal. A Cardinal is a 32 bit type, but IcmpCreateFile returns HANDLE which is a pointer sized type. Your code will fail when compiled on 64 bit. Some of the other types that you use in your IcmpSendEcho header translation look dubious. If I were you I would either bone up on how to translate Win32 header files, and find and use trustworthy header translations.

I would also like to stress that your code makes no attempt whatsoever to handle errors. You are playing with fire here. We see this so many times here on SO, and I am sure I have personally told you this very thing on a number of early occasions. So, read the documentation for each API function that you call, and handle errors appropriately.

Unless you are an expert in Win32 API header translations and use, then you are likely to produce code that has defects. It might be prudent to use a third party header translation from, for instance, JEDI.

Actually I used tHandle but when I moved a mouse pointer on it the pop-up comment showed it was a cardinal. So I changed and thought delphi recognized it as a cardinal internally. Because it's only a skeleton for understanding the method I didn't use any exception handler. I deeply understand your advice. I don't use indy because some users who will use this even don't understand what the administrator authority. This annoying me either. Always thanks. — JO SeongGng, Feb 29 '16 at 17:53
That's a mistake. On 32 bit it maps to Cardinal, but on 64 bit it maps to UInt64. The Win32 type `HANDLE` should map to `THandle`. — David Heffernan, Feb 29 '16 at 17:54
Ugh... As stated in the Q, Indy requires raw sockets privilege and thus sucks at ICMP. @JOSeongGng, you could use JEDI WINAPI units. — Free Consulting, Mar 01 '16 at 00:52

Should I close a handle with IcmpCloseHandle?

1 Answers1