Add, Remove Folder from IShellLibrary

I am attempting to write two functions that add and remove a folder from a IShellLibrary. I started with this, but the function produces an exception in System._IntfClear:

First chance exception at $000007FEFE 168BC4. Exception class $C0000005 with Message 'c0000005 ACCESS_VIOLATION'.

The SHAddFolderPathToLibrary is the line that causes the exception.

I guess I need to add the library name to the function?

function AddFolderToLibrary(AFolder: string): HRESULT;
{ Add AFolder to Windows 7 library. }
var
  plib: IShellLibrary;
begin
  Result := CoCreateInstance(CLSID_ShellLibrary, nil, CLSCTX_INPROC_SERVER,
    IID_IShellLibrary, plib);
  if SUCCEEDED(Result) then
  begin
    Result := SHAddFolderPathToLibrary(plib, PWideChar(AFolder));
  end;
end;

function RemoveFolderFromLibrary(AFolder: string): HRESULT;
{ Remove AFolder from Windows 7 library. }
var
  plib: IShellLibrary;
begin
  Result := CoCreateInstance(CLSID_ShellLibrary, nil, CLSCTX_INPROC_SERVER,
    IID_IShellLibrary, plib);
  if SUCCEEDED(Result) then
  begin
    Result := SHRemoveFolderPathFromLibrary(plib, PWideChar(AFolder));
  end;
end;

The problem here is that the Embarcadero engineer who translated SHAddFolderPathToLibrary does not understand COM reference counting, and how it is handled by different compilers.

Here's how SHAddFolderPathToLibrary is implemented in the C++ header file Shobjidl.h. It's actually an inline wrapper of other core API calls:

__inline HRESULT SHAddFolderPathToLibrary(_In_ IShellLibrary *plib, 
    _In_ PCWSTR pszFolderPath)
{
    IShellItem *psiFolder;
    HRESULT hr = SHCreateItemFromParsingName(pszFolderPath, NULL, 
        IID_PPV_ARGS(&psiFolder));
    if (SUCCEEDED(hr))
    {
        hr = plib->AddFolder(psiFolder);
        psiFolder->Release();
    }
    return hr;
}

And the Delphi translation is very faithful, indeed too faithful:

function SHAddFolderPathToLibrary(const plib: IShellLibrary;
  pszFolderPath: LPCWSTR): HResult;
var
  psiFolder: IShellItem;
begin
  Result := SHCreateItemFromParsingName(pszFolderPath, nil, IID_IShellItem,
    psiFolder);
  if Succeeded(Result) then
  begin
    Result := plib.AddFolder(psiFolder);
    psiFolder._Release();
  end;
end;

The problem is the call to _Release. The Delphi compiler manages reference counting, and so this explicit call to _Release is bogus and should not be there. Since the compiler will arrange for a call to _Release, this extra one simply unbalances the reference counting. The reason why _AddRef and _Release are prefixed with _ is to remind people not to call them and to let the compiler do that.

The call to Release in the C++ version is accurate because C++ compilers don't automatically call Release for you unless you wrap the interface in a COM smart pointer. But the Embarcadero engineer has blindly copied it across and you are left with the consequences. Clearly this code has never even been executed by the Embarcadero engineers.

You'll need to supply your own corrected implementation of this function. And also any other erroneously translated function. Search for _Release in the ShlObj unit, and remove them in your corrected versions. There are other bugs in the translation, so watch out. For example, SHLoadLibraryFromItem (and others) declare local variable plib: ^IShellLibrary which should be plib: IShellLibrary.

I submitted a QC report: QC#117351.