Freeing memory?

I am not hung up on the idea that the code in the support section of MSDN be perfect, but I want my code to be as correct as I can get it. The code
below is from MSDN. There is a call to RtlAnsiStringToUnicodeString with TRUE being passed to allocate the destination buffer, but I never see it
free’d. Is this a mistake or is it getting free’d somewhere in another call that I just don’t know about?

{
ANSI_STRING ntString;
UNICODE_STRING ntUnicodeString;
KEVENT event;
PDEVICE_OBJECT PcmciaDeviceObject = NULL;
NTSTATUS istatus = 0;
PFILE_OBJECT fileObject = NULL;
PIRP irp = NULL;
LONG SktNo = 0;
LONG ValidSkts = 0;
PCMCIA_SOCKET_INFORMATION PcmciaSkt;
IO_STATUS_BLOCK ioStatusBlock;
CCHAR deviceNameBuffer[64];
LONG portNumber = 0;
LONG MorePorts = TRUE;
LONG MoreSkts = TRUE;

//Loop for all the controllers.
do{
sprintf(deviceNameBuffer,“\Device\Pcmcia%d”,portNumber);
RtlInitAnsiString(
&ntString,
deviceNameBuffer
); //Initialize the string
istatus = RtlAnsiStringToUnicodeString(
&ntUnicodeString,
&ntString,
TRUE
);
if (!NT_SUCCESS(istatus)){
//Handle this error properly.
return STATUS_UNSUCCESSFUL; //Error
}//ENDIF
status = IoGetDeviceObjectPointer(
&ntUnicodeString,
FILE_READ_ATTRIBUTES,
&fileObject,
&PcmciaDeviceObject
);
if (!NT_SUCCESS(istatus)){
return STATUS_UNSUCCESSFUL; //Error
}//ENDIF
do{
RtlZeroMemory(
&PcmciaSkt,
sizeof(PCMCIA_SOCKET_INFORMATION)
);
PcmciaSkt.Socket = (USHORT) SktNo;
KeInitializeEvent(&event, NotificationEvent, FALSE);
irp = IoBuildDeviceIoControlRequest
(
IOCTL_SOCKET_INFORMATION,
PcmciaDeviceObject,
&PcmciaSkt,
sizeof(PCMCIA_SOCKET_INFORMATION),
&PcmciaSkt,
sizeof (PCMCIA_SOCKET_INFORMATION),
FALSE,
&event,
&ioStatusBlock
);

if (!irp){
//Handle the error properly.
MoreSkts = FALSE;
break; //Error
}//ENDIF

ioStatusBlock.Status = STATUS_NOT_SUPPORTED;
ioStatusBlock.Information = 0;

// Send the request down.
istatus = IoCallDriver(
PcmciaDeviceObject,
irp
);
if (istatus == STATUS_PENDING){
// If pending then wait for done.
KeWaitForSingleObject(
&event,
Executive,
KernelMode,
FALSE,
NULL
);
istatus = ioStatusBlock.Status;
}//ENDIF
if (!NT_SUCCESS(istatus)){
//The error can occur for many reasons including the
//STATUS_INVALID_PARAMETER,STATUS_BUFFER_TOO_SMALL
//if the parameters in the IoBuildDevice… function
//are not valid.

//If the error is because of invalid parameter then,
//it could be because there may not be more valid socket
//on this controller. In this case just break see if
//any other controller exists.
//Otherwise handle the error carefully.

MoreSkts = FALSE;
break;
}//ENDIF
//You have the socket information now to handle in your driver.
SktNo++;
}while (MoreSkts)//END FOR
//Dereference the file object on this controller.
ObDereferenceObject(fileObject);
portNumber++;
//Here before reinitializing the flag you can insert your code.
MoreSkts = TRUE;
}while(MorePorts)//END WHILE
MorePorts = TRUE;
}

>Is this a mistake or is it getting free’d somewhere in another call that I just don’t know about?
If a buffer allocated by RtlAnsiStringToUnicodeString, using AllocateDestinationString=TRUE, it should be freed. It likely done either in Unload or a routine which destroys internal data belong to particular Device Object.

Igor Sharovar

And it could be a memory leak in that particular sample. It would not be the first bug found in WDDK samples/examples.

Gary G. Little

----- Original Message -----
From: “igor sharovar”
To: “Windows System Software Devs Interest List”
Sent: Wednesday, January 5, 2011 12:55:08 PM
Subject: RE:[ntdev] Freeing memory?

>Is this a mistake or is it getting free’d somewhere in another call that I just don’t know about?
If a buffer allocated by RtlAnsiStringToUnicodeString, using AllocateDestinationString=TRUE, it should be freed. It likely done either in Unload or a routine which destroys internal data belong to particular Device Object.

Igor Sharovar


NTDEV is sponsored by OSR

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer

note that this entire example in terms of how to use printf is completely borked. You should use RtlUnicodeStringPrintf instead. no allocation, no conversion from ansi to unicode, no intializing a UNICODE_STRING after initializing a null terminated C style string.

d

>RtlUnicodeStringPrintf instead

RtlStringCbPrintfW also helps a lot.


Maxim S. Shatskih
Windows DDK MVP
xxxxx@storagecraft.com
http://www.storagecraft.com

If the “destination” of the buffer is be put into a UNICODE_STRING, RtlStringCbXxx (orCch) makes no sense, just use the RtlUnicodeXxx equivalents

d

Aside from the other comments indicating that there are much better
interfaces, the docs for RtlAnsiStringToUnicodeString are quite
explicit about the caller being required to call RtlFreeUnicodeString
if AllocateDestinationString is TRUE.

Code samples, even for functions as useless as this one, ought to be correct.

Mark Roddy

On Wed, Jan 5, 2011 at 1:11 PM, Michael Wade wrote:
> I am not hung up on the idea that the code in the support section of MSDN be perfect, but I want my code to be as correct as I can get it. ?The code
> below is from MSDN. ?There is a call to RtlAnsiStringToUnicodeString with TRUE being passed to allocate the destination buffer, but I never see it
> free’d. ?Is this a mistake or is it getting free’d somewhere in another call that I just don’t know about?
>
> {
> ? ?ANSI_STRING ntString;
> ? ?UNICODE_STRING ntUnicodeString;
> ? ?KEVENT event;
> ? ?PDEVICE_OBJECT PcmciaDeviceObject = NULL;
> ? ?NTSTATUS istatus = 0;
> ? ?PFILE_OBJECT fileObject = NULL;
> ? ?PIRP irp = NULL;
> ? ?LONG SktNo = 0;
> ? ?LONG ValidSkts = 0;
> ? ?PCMCIA_SOCKET_INFORMATION PcmciaSkt;
> ? ?IO_STATUS_BLOCK ioStatusBlock;
> ? ?CCHAR deviceNameBuffer[64];
> ? ?LONG portNumber = 0;
> ? ?LONG MorePorts = TRUE;
> ? ?LONG MoreSkts = TRUE;
>
> ? ?//Loop for all the controllers.
> ? ?do{
> ? ? ? ?sprintf(deviceNameBuffer,“\Device\Pcmcia%d”,portNumber);
> ? ? ? ?RtlInitAnsiString(
> ? ? ? ? ? ? ? ? ? ? ? ? ?&ntString,
> ? ? ? ? ? ? ? ? ? ? ? ? ?deviceNameBuffer
> ? ? ? ? ? ? ? ? ? ? ? ? ); //Initialize the string
> ? ? ? ?istatus = RtlAnsiStringToUnicodeString(
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &ntUnicodeString,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &ntString,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TRUE
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?);
> ? ? ? ?if (!NT_SUCCESS(istatus)){
> ? ? ? ? ? ?//Handle this error properly.
> ? ? ? ? ? ?return STATUS_UNSUCCESSFUL; //Error
> ? ? ? ?}//ENDIF
> ? ? ? ?status = IoGetDeviceObjectPointer(
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&ntUnicodeString,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?FILE_READ_ATTRIBUTES,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&fileObject,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&PcmciaDeviceObject
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? );
> ? ? ? ?if (!NT_SUCCESS(istatus)){
> ? ? ? ? ? ?return STATUS_UNSUCCESSFUL; //Error
> ? ? ? ?}//ENDIF
> ? ? ? ?do{
> ? ? ? ? ? ?RtlZeroMemory(
> ? ? ? ? ? ? ? ? ? ? ? ? ?&PcmciaSkt,
> ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(PCMCIA_SOCKET_INFORMATION)
> ? ? ? ? ? ? ? ? ? ? ? ? );
> ? ? ? ? ? ?PcmciaSkt.Socket = (USHORT) SktNo;
> ? ? ? ? ? ?KeInitializeEvent(&event, NotificationEvent, FALSE);
> ? ? ? ? ? ?irp = IoBuildDeviceIoControlRequest
> ? ? ? ? ? ? ? ? ? ? ? ?(
> ? ? ? ? ? ? ? ? ? ? ? ? IOCTL_SOCKET_INFORMATION,
> ? ? ? ? ? ? ? ? ? ? ? ? PcmciaDeviceObject,
> ? ? ? ? ? ? ? ? ? ? ? ? &PcmciaSkt,
> ? ? ? ? ? ? ? ? ? ? ? ? sizeof(PCMCIA_SOCKET_INFORMATION),
> ? ? ? ? ? ? ? ? ? ? ? ? &PcmciaSkt,
> ? ? ? ? ? ? ? ? ? ? ? ? sizeof (PCMCIA_SOCKET_INFORMATION),
> ? ? ? ? ? ? ? ? ? ? ? ? FALSE,
> ? ? ? ? ? ? ? ? ? ? ? ? &event,
> ? ? ? ? ? ? ? ? ? ? ? ? &ioStatusBlock
> ? ? ? ? ? ? ? ? ? ? ? ?);
>
> ? ? ? ? ? ?if (!irp){
> ? ? ? ? ? ? ? ?//Handle the error properly.
> ? ? ? ? ? ? ? ?MoreSkts = FALSE;
> ? ? ? ? ? ? ? ?break; //Error
> ? ? ? ? ? ?}//ENDIF
>
> ? ? ? ? ? ?ioStatusBlock.Status = STATUS_NOT_SUPPORTED;
> ? ? ? ? ? ?ioStatusBlock.Information = 0;
>
> ? ? ? ? ? ?// Send the request down.
> ? ? ? ? ? ?istatus = IoCallDriver(
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?PcmciaDeviceObject,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?irp
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? );
> ? ? ? ? ? ?if (istatus == STATUS_PENDING){
> ? ? ? ? ? ?// If pending then wait for done.
> ? ? ? ? ? ? ? ?KeWaitForSingleObject(
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&event,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Executive,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?KernelMode,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?FALSE,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?NULL
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? );
> ? ? ? ? ? ? ? ?istatus = ioStatusBlock.Status;
> ? ? ? ? ? ?}//ENDIF
> ? ? ? ? ? ?if (!NT_SUCCESS(istatus)){
> ? ? ? ? ? ? ? ?//The error can occur for many reasons including the
> ? ? ? ? ? ? ? ?//STATUS_INVALID_PARAMETER,STATUS_BUFFER_TOO_SMALL
> ? ? ? ? ? ? ? ?//if the parameters in the IoBuildDevice… function
> ? ? ? ? ? ? ? ?//are not valid.
>
> ? ? ? ? ? ? ? ?//If the error is because of invalid parameter then,
> ? ? ? ? ? ? ? ?//it could be because there may not be more valid socket
> ? ? ? ? ? ? ? ?//on this controller. In this case just break see if
> ? ? ? ? ? ? ? ?//any other controller exists.
> ? ? ? ? ? ? ? ?//Otherwise handle the error carefully.
>
> ? ? ? ? ? ? ? ?MoreSkts = FALSE;
> ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ?}//ENDIF
> ? ? ? ? ? ?//You have the socket information now to handle in your driver.
> ? ? ? ? ? ?SktNo++;
> ? ? ? ?}while (MoreSkts)//END FOR
> ? ? ? ?//Dereference the file object on this controller.
> ? ? ? ?ObDereferenceObject(fileObject);
> ? ? ? ?portNumber++;
> ? ? ? ?//Here before reinitializing the flag you can insert your code.
> ? ? ? ?MoreSkts = TRUE;
> ? ?}while(MorePorts)//END WHILE
> ? ?MorePorts = TRUE;
> }
>
> —
> NTDEV is sponsored by OSR
>
> For our schedule of WDF, WDM, debugging and other seminars visit:
> http://www.osr.com/seminars
>
> To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer
>

I would like to be a good citizen of the driver development community. Is this the sort of thing that I should report to Microsoft?

On Fri, 7 Jan 2011 09:09:17 -0500, Mark Roddy wrote:

>Aside from the other comments indicating that there are much better
>interfaces, the docs for RtlAnsiStringToUnicodeString are quite
>explicit about the caller being required to call RtlFreeUnicodeString
>if AllocateDestinationString is TRUE.
>
>Code samples, even for functions as useless as this one, ought to be correct.
>
>Mark Roddy
>
>
>
>On Wed, Jan 5, 2011 at 1:11 PM, Michael Wade wrote:
>> I am not hung up on the idea that the code in the support section of MSDN be perfect, but I want my code to be as correct as I can get it. The code
>> below is from MSDN. There is a call to RtlAnsiStringToUnicodeString with TRUE being passed to allocate the destination buffer, but I never see it
>> free’d. Is this a mistake or is it getting free’d somewhere in another call that I just don’t know about?
>>
>> {
>> ANSI_STRING ntString;
>> UNICODE_STRING ntUnicodeString;
>> KEVENT event;
>> PDEVICE_OBJECT PcmciaDeviceObject = NULL;
>> NTSTATUS istatus = 0;
>> PFILE_OBJECT fileObject = NULL;
>> PIRP irp = NULL;
>> LONG SktNo = 0;
>> LONG ValidSkts = 0;
>> PCMCIA_SOCKET_INFORMATION PcmciaSkt;
>> IO_STATUS_BLOCK ioStatusBlock;
>> CCHAR deviceNameBuffer[64];
>> LONG portNumber = 0;
>> LONG MorePorts = TRUE;
>> LONG MoreSkts = TRUE;
>>
>> //Loop for all the controllers.
>> do{
>> sprintf(deviceNameBuffer,“\Device\Pcmcia%d”,portNumber);
>> RtlInitAnsiString(
>> &ntString,
>> deviceNameBuffer
>> ); //Initialize the string
>> istatus = RtlAnsiStringToUnicodeString(
>> &ntUnicodeString,
>> &ntString,
>> TRUE
>> );
>> if (!NT_SUCCESS(istatus)){
>> //Handle this error properly.
>> return STATUS_UNSUCCESSFUL; //Error
>> }//ENDIF
>> status = IoGetDeviceObjectPointer(
>> &ntUnicodeString,
>> FILE_READ_ATTRIBUTES,
>> &fileObject,
>> &PcmciaDeviceObject
>> );
>> if (!NT_SUCCESS(istatus)){
>> return STATUS_UNSUCCESSFUL; //Error
>> }//ENDIF
>> do{
>> RtlZeroMemory(
>> &PcmciaSkt,
>> sizeof(PCMCIA_SOCKET_INFORMATION)
>> );
>> PcmciaSkt.Socket = (USHORT) SktNo;
>> KeInitializeEvent(&event, NotificationEvent, FALSE);
>> irp = IoBuildDeviceIoControlRequest
>> (
>> IOCTL_SOCKET_INFORMATION,
>> PcmciaDeviceObject,
>> &PcmciaSkt,
>> sizeof(PCMCIA_SOCKET_INFORMATION),
>> &PcmciaSkt,
>> sizeof (PCMCIA_SOCKET_INFORMATION),
>> FALSE,
>> &event,
>> &ioStatusBlock
>> );
>>
>> if (!irp){
>> //Handle the error properly.
>> MoreSkts = FALSE;
>> break; //Error
>> }//ENDIF
>>
>> ioStatusBlock.Status = STATUS_NOT_SUPPORTED;
>> ioStatusBlock.Information = 0;
>>
>> // Send the request down.
>> istatus = IoCallDriver(
>> PcmciaDeviceObject,
>> irp
>> );
>> if (istatus == STATUS_PENDING){
>> // If pending then wait for done.
>> KeWaitForSingleObject(
>> &event,
>> Executive,
>> KernelMode,
>> FALSE,
>> NULL
>> );
>> istatus = ioStatusBlock.Status;
>> }//ENDIF
>> if (!NT_SUCCESS(istatus)){
>> //The error can occur for many reasons including the
>> //STATUS_INVALID_PARAMETER,STATUS_BUFFER_TOO_SMALL
>> //if the parameters in the IoBuildDevice… function
>> //are not valid.
>>
>> //If the error is because of invalid parameter then,
>> //it could be because there may not be more valid socket
>> //on this controller. In this case just break see if
>> //any other controller exists.
>> //Otherwise handle the error carefully.
>>
>> MoreSkts = FALSE;
>> break;
>> }//ENDIF
>> //You have the socket information now to handle in your driver.
>> SktNo++;
>> }while (MoreSkts)//END FOR
>> //Dereference the file object on this controller.
>> ObDereferenceObject(fileObject);
>> portNumber++;
>> //Here before reinitializing the flag you can insert your code.
>> MoreSkts = TRUE;
>> }while(MorePorts)//END WHILE
>> MorePorts = TRUE;
>> }
>>
>> —
>> NTDEV is sponsored by OSR
>>
>> For our schedule of WDF, WDM, debugging and other seminars visit:
>> http://www.osr.com/seminars
>>
>> To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer
>>