Watch, Follow, &
Connect with Us

For forums, blogs and more please visit our
Developer Tools Community.


Welcome, Guest
Guest Settings
Help

Thread: Freeing memory when using IShellFolder::GetDisplayNameOf


This question is answered.


Permlink Replies: 2 - Last Post: Nov 8, 2016 2:14 PM Last Post By: Asger Joergensen
Asger Joergensen

Posts: 370
Registered: 11/18/08
Freeing memory when using IShellFolder::GetDisplayNameOf  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Nov 8, 2016 10:58 AM
Hi

I have this code in many old examples and aps. Everywhere I find the line with the //<<< is missing.
The help say: It is the caller's responsibility to free resources allocated by GetDisplayNameOf
And the only thing I can see that is allocated is the pOleStr.

My question is: Am I right that the marked line should be there ??

Thanks in advance
Best regards
Asger

 String GetDisplayName(LPSHELLFOLDER ShellFolder, PItemIDList PIDL, bool ForParsing)
{
  TStrRet StrRet = {0};
   char*  P;
   int Flags;
   String Result;
 
   if( ForParsing )
       Flags = SHGDN_FORPARSING;
   else
       Flags = SHGDN_NORMAL;
 
   ShellFolder->GetDisplayNameOf( PIDL, Flags, &StrRet );
 
   switch( StrRet.uType )
   {
       case STRRET_CSTR:       Result = String( StrRet.cStr, strlen( StrRet.cStr ) );
                               break;
       case STRRET_OFFSET:     P = &PIDL->mkid.abID[StrRet.uOffset - sizeof( PIDL->mkid.cb )];
                               Result = String( P );
                               break;
       case STRRET_WSTR:       Result = String( StrRet.pOleStr );
                               CoTaskMemFree( StrRet.pOleStr );  //<<<
                               break;
   }
   return Result;
}
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: Freeing memory when using IShellFolder::GetDisplayNameOf
Correct
Click to report abuse...   Click to reply to this thread Reply
  Posted: Nov 8, 2016 12:21 PM   in response to: Asger Joergensen in response to: Asger Joergensen
Asger wrote:

The help say: It is the caller's responsibility to free resources
allocated by GetDisplayNameOf
And the only thing I can see that is allocated is the pOleStr.

Correct.

My question is: Am I right that the marked line should be there ??

Yes, and this is clearly stated in the STRRET documentation:

pOleStr
Type: LPWSTR

A pointer to the string. This memory must be allocated with CoTaskMemAlloc.
It is the calling application's responsibility to free this memory with CoTaskMemFree
when it is no longer needed.

TStrRet StrRet = {0};

Since you are using other Win32 API types directly, you should use the Win32
API STRRET directly as well, instead of the Delphi TStrRet typedef.

ShellFolder->GetDisplayNameOf( PIDL, Flags, &StrRet );

You are not checking the return value of GetDisplayNameOf(). The STRRET
content is only valid if GetDisplayNameOf() returns S_OK.

switch( StrRet.uType )
{
case STRRET_CSTR: Result = String( StrRet.cStr, strlen( StrRet.cStr ) );
break;
case STRRET_OFFSET: P = &PIDL->mkid.abID[StrRet.uOffset - sizeof( PIDL->mkid.cb
)];
Result = String( P );
break;
case STRRET_WSTR: Result = String( StrRet.pOleStr );
CoTaskMemFree( StrRet.pOleStr ); //<<<
break;
}

Rather than doing all of that manually, you should do what the GetDisplayNameOf()
documentation suggests:

The simplest way to retrieve the display name from the structure pointed
to by pName is to pass it to either StrRetToBuf or StrRetToStr. These functions
take a STRRET structure and return the name.

Not only do they handle the various types for you, but they also free the
pOleStr when uType is STRRET_WSTR.

Try something more like this instead:

String GetDisplayName(LPSHELLFOLDER ShellFolder, PItemIDList PIDL, bool ForParsing)
{
    STRRET StrRet;
    SHGDNF Flags = ForParsing ? SHGDN_FORPARSING : SHGDN_NORMAL;
    String Result;
 
    HRESULT hr = ShellFolder->GetDisplayNameOf( PIDL, Flags, &StrRet );
    if ( SUCCEEDED(hr) )
    {
        #ifdef _DELPHI_STRING_UNICODE
        LPWSTR pName;
        hr = StrRetToStrW( &StrRet, PIDL, &pName );
        #else
        LPSTR pName;
        hr = StrRetToStrA( &StrRet, PIDL, &pName );
        #endif
        if ( SUCCEEDED(hr) )
        {
            try {
                Result = pName;
            }
            __finally {
                CoTaskMemFree(pName);
            }
        }
 
        /*
        alternatively:

        WideString wName;
        hr = StrRetToBSTR( &StrRet, PIDL, &wName );
        if ( SUCCEEDED(hr) )
            Result = wName;
        */
    }
 
    return Result;
}


--
Remy Lebeau (TeamB)
Asger Joergensen

Posts: 370
Registered: 11/18/08
Re: Freeing memory when using IShellFolder::GetDisplayNameOf  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Nov 8, 2016 2:14 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Thank you very much Remy

That is a lot more elegant.

Best regards
Asger
Legend
Helpful Answer (5 pts)
Correct Answer (10 pts)

Server Response from: ETNAJIVE02