ExtractIconEx: works but occasionally crashes

user_1136512 0
Viewed 189

I'm extracting icons from a file and displaying them in a dialog

const LPCWSTR path = L"c:\path\to\file";
const UINT nIconsCheck = ExtractIconEx(path, -1, nullptr, nullptr, 0);
if(nIconsCheck > 0)
{
    HICON *iconHandles=new HICON;
    const UINT nIcons = ExtractIconEx(path, 0, iconHandles, nullptr, nIconsCheck);

    if(nIcons == nIconsCheck && nIcons != unsigned(-1))
    {

        IconSelect iconSelect(this); //dialog
        for(UINT i=0; i<nIcons; i++)
        {
            qDebug() << i;
            iconSelect.addIcon(QtWin::fromHICON(iconHandles[i])); //fromHICON returns QPixmap
            DestroyIcon(iconHandles[i]);
        }

        iconSelect.exec();
    }
}

The icons are being loaded correctly in the dialog, but sometimes it unpredictably causes the application to crash.

Any idea what is going on?

Documentation on ExtractIconEx

Edit: Thanks for the quick and helpful answers. Below is the complete working code I am using atm:

// In my case I have a QString `filePath`
// `QString::toWCharArray` retrieves a non-0-terminated string,
// so append a 0 to `path`
std::vector<WCHAR> path(unsigned(filePath.length())+1); 
filePath.toWCharArray(path.data());
path.at(path.size()-1) = 0;

// Get number of icons in selected file
UINT nIcons = ExtractIconEx(path.data(), -1, nullptr, nullptr, 0);

if(nIcons == 0)
{
    // Try to find associated file that contains icon(s)
    // If found, `path` is replaced with the new path
    WORD index=0;
    DestroyIcon(ExtractAssociatedIcon(GetModuleHandle(nullptr), path.data(), &index));
    // Get number of icons in associated file
    nIcons = ExtractIconEx(path.data(), -1, nullptr, nullptr, 0);
}

if(nIcons > 0)
{
    // Get array of HICONs
    std::vector<HICON> iconHandles(nIcons);
    nIcons = ExtractIconEx(path.data(), 0, iconHandles.data(), nullptr, nIcons);

    for(UINT i=0; i<nIcons; i++) // Using iconHandles.size() is possibly safer,
                                 // but AFAIK nIcons always carries the correct value
    {
        // Use iconHandles[i]
        // In Qt you can use QtWin::fromHICON(iconHandles[i]) to generate a QPixmap
        DestroyIcon(iconHandles[i]);
    }
}
icons allocation winapi arrays c++
2 Answers
HICON *iconHandles=new HICON;

Here you are allocating only a single HICON object. If there are more than one icons in the given file, the next call to ExtractIconEx() creates a buffer overrun by writing past the allocated memory. You have entered the dark world of undefined behaviour.

To fix this problem, you could use a std::vector like this:

std::vector<HICON> iconHandles(nIconsCheck); 
const UINT nIcons = ExtractIconEx(path, 0, iconHandles.data(), nullptr, iconHandles.size());
iconHandles.resize(nIcons); // Resize to the actual number of icons.

// Instead of: if(nIcons == nIconsCheck && nIcons != unsigned(-1))
if(!iconHandles.empty())
{
    // Use icons
}

This has the advantage over manual allocation, that you don't need to delete the allocated memory. The vector destructor will do it automatically when the scope ends. Though you still have to call DestroyIcon() for each icon handle.

user_75712580

From the documentation you linked to:

Pointer to an array of icon handles that receives handles to the large icons extracted from the file. If this parameter is NULL, no large icons are extracted from the file.

You only gave it a pointer to one icon handle.

Allocate an array as large as the function expects; from the look of it, that means nIconsCheck elements. A vector is good for this, as zett42 says.

user_5606480
Related