Re: [PATCH 2/3] winex11.drv: copy and paste files to/from X11 apps (try 2)

[Alexandre Julliard]

Damjan Jovanovic <damjan.jov(a)gmail.com> writes:
> /**************************************************************************
> + *      X11DRV_CLIPBOARD_ExportHDROP
> + *
> + *  Export CF_HDROP format to text/uri-list.
> + */
> +static HANDLE X11DRV_CLIPBOARD_ExportHDROP(Display *display, Window requestor, Atom aTarget,
> +    Atom rprop, LPWINE_CLIPDATA lpdata, LPDWORD lpBytes)
> +{
> +    HDROP hDrop;
> +    UINT i;
> +    UINT numFiles;
> +    UINT totalSize = 0;
> +    HGLOBAL hClipData = NULL;
> +    char *text_uri_list = NULL;
> +    char *next;
> +    BOOLEAN succeeded = FALSE;
> +
> +    *lpBytes = 0;
> +
> +    if (!X11DRV_CLIPBOARD_RenderFormat(display, lpdata))
> +    {
> +        ERR("Failed to export %04x format\n", lpdata->wFormatID);
> +        return 0;
> +    }
> +
> +    hDrop = (HDROP) lpdata->hData;
> +    numFiles = DragQueryFileW(hDrop, 0xFFFFFFFF, NULL, 0);
> +
> +    for (i = 0; i < numFiles; i++)
> +    {
> +        UINT dosFilenameSize;
> +        WCHAR *dosFilename;
> +        succeeded = FALSE;
> +        dosFilenameSize = 1 + DragQueryFileW(hDrop, i, NULL, 0);
> +        dosFilename = HeapAlloc(GetProcessHeap(), 0, dosFilenameSize*sizeof(WCHAR));
> +        if (dosFilename)
> +        {
> +            char *unixFilename;
> +            DragQueryFileW(hDrop, i, dosFilename, dosFilenameSize);
> +            unixFilename = wine_get_unix_file_name(dosFilename);
> +            if (unixFilename)
> +            {
> +                totalSize += 8 + /* file:/// */
> +                        3*(lstrlenA(unixFilename) - 1); /* "%xy" per char except first '/' */
> +                succeeded = TRUE;
> +                HeapFree(GetProcessHeap(), 0, unixFilename);
> +            }
> +            HeapFree(GetProcessHeap(), 0, dosFilename);
> +        }
> +        if (!succeeded)
> +        {
> +            ERR("out of memory\n");
> +            goto end;
> +        }
> +    }
> +    totalSize += 2 * numFiles; /* "\r\n" */
> +    hClipData = GlobalAlloc(GMEM_MOVEABLE | GMEM_DDESHARE, totalSize);
> +    if (hClipData == NULL)
> +    {
> +        ERR("out of memory\n");
> +        succeeded = FALSE;
> +        goto end;
> +    }
> +    text_uri_list = (char*) GlobalLock(hClipData);
> +    next = text_uri_list;
> +    for (i = 0; i < numFiles; i++)
> +    {
> +        UINT dosFilenameSize;
> +        WCHAR *dosFilename;
> +        succeeded = FALSE;
> +        dosFilenameSize = 1 + DragQueryFileW(hDrop, i, NULL, 0);
> +        dosFilename = HeapAlloc(GetProcessHeap(), 0, dosFilenameSize*sizeof(WCHAR));
> +        if (dosFilename)
> +        {
> +            char *unixFilename;
> +            DragQueryFileW(hDrop, i, dosFilename, dosFilenameSize);
> +            unixFilename = wine_get_unix_file_name(dosFilename);
> +            if (unixFilename)
> +            {
> +                static const char *hex_table = "0123456789abcdef";
> +                UINT u;
> +                *next++ = 'f';
> +                *next++ = 'i';
> +                *next++ = 'l';
> +                *next++ = 'e';
> +                *next++ = ':';
> +                *next++ = '/';
> +                *next++ = '/';
> +                *next++ = '/';
> +                /* URL encode everything - unnecessary, but easier/lighter than linking in shlwapi, and can't hurt */
> +                for (u = 1; unixFilename[u]; u++) {
> +                    *next++ = '%';
> +                    *next++ = hex_table[unixFilename[u] >> 4];
> +                    *next++ = hex_table[unixFilename[u] & 0xf];
> +                }
> +                *next++ = '\r';
> +                *next++ = '\n';
> +                succeeded = TRUE;
> +                HeapFree(GetProcessHeap(), 0, unixFilename);
> +            }
> +            HeapFree(GetProcessHeap(), 0, dosFilename);
> +        }
> +        if (!succeeded)
> +        {
> +            ERR("out of memory\n");
> +            goto end;
> +        }
> +    }
> +
> +end:
> +    if (succeeded)
> +    {
> +        GlobalUnlock(hClipData);
> +        *lpBytes = GlobalSize(hClipData);
> +        return hClipData;
> +    }
> +    else
> +    {
> +        if (text_uri_list)
> +            GlobalUnlock(hClipData);
> +        GlobalFree(hClipData);
> +        *lpBytes = 0;
> +        return 0;
> +    }
> +}

That's unnecessarily complicated and inefficient. You don't need to
retrieve the file name 4 times, or to convert every filename to Unix
twice.

-- 
Alexandre Julliard
julliard(a)winehq.org