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

[Alexandre Julliard]

Damjan Jovanovic  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.