Re: [PATCH] winemapi: Directly use xdg-email if available, enabling file attachments.

[Hans Leidekker]

Hi Jeremy,
> diff --git a/dlls/winemapi/Makefile.in b/dlls/winemapi/Makefile.in
> index 813ee42..d6f79b4 100644
> --- a/dlls/winemapi/Makefile.in
> +++ b/dlls/winemapi/Makefile.in
> @@ -3,4 +3,5 @@ IMPORTS   = shlwapi shell32 mapi32
>
> C_SRCS = \
>   main.c \
> -	sendmail.c
> +	sendmail.c \
> +	xdg-email.c

I think it would be better to put this in sendmail.c. There’s not enough code to warrant a new file and
I don’t expect it to grow.
> +/**************************************************************************
> + *  MAPISendMail
> + *
> + * Send a message using a native mail client.
> + *
> + * PARAMS
> + *  session  [I] Handle to a MAPI session.
> + *  uiparam  [I] Parent window handle.
> + *  message  [I] Pointer to a MAPIMessage structure.
> + *  flags    [I] Flags.
> + *  reserved [I] Reserved, pass 0.
> + *
> + * RETURNS
> + *  Success: SUCCESS_SUCCESS
> + *  Failure: MAPI_E_FAILURE
> + *
> + */
> +ULONG WINAPI MAPISendMail(LHANDLE session, ULONG_PTR uiparam,
> +    lpMapiMessage message, FLAGS flags, ULONG reserved)
> +{
> +    if (XDGMailAvailable())
> +        return XDGSendMail(session, uiparam, message, flags, reserved);
> +
> +    return BrowserSendMail(session, uiparam, message, flags, reserved);
> +}

It’s hard to reliably check for availability of any given command, and even if you can there’s still a race.
You should just call _spawnvp and fall back to the existing path if it fails.
> +static void add_argument(char **argv, int *argc, const char *arg, const char *param)
> +{
> +    argv[(*argc)] = HeapAlloc(GetProcessHeap(), 0, strlen(arg) + 1);
> +    strcpy(argv[(*argc)++], arg);
> +    if (param)
> +    {
> +        argv[(*argc)] = HeapAlloc(GetProcessHeap(), 0, strlen(param) + 1);
> +        strcpy(argv[(*argc)++], param);
> +    }
> +}
> +
> +static void add_target(char **argv, int *argc, ULONG class, const char *address)
> +{
> +    static const char smtp[] = "smtp:";
> +
> +    if (!strncasecmp(address, smtp, sizeof(smtp) - 1))
> +        address += sizeof(smtp) - 1;
> +
> +    switch (class)
> +    {
> +        case MAPI_ORIG:
> +            TRACE("From: %s\n (unused)", debugstr_a(address));
> +            break;
> +
> +        case MAPI_TO:
> +            TRACE("To: %s\n", debugstr_a(address));
> +            add_argument(argv, argc, address, NULL);
> +            break;
> +
> +        case MAPI_CC:
> +            TRACE("CC: %s\n", debugstr_a(address));
> +            add_argument(argv, argc, "--cc", address);
> +            break;
> +
> +        case MAPI_BCC:
> +            TRACE("BCC: %s\n", debugstr_a(address));
> +            add_argument(argv, argc, "--bcc", address);
> +            break;
> +
> +        default:
> +            TRACE("Unknown recipient class: %d\n", class);
> +    }
> +}
> +
> +static void add_file(char **argv, int *argc, const char *path, const char *file)
> +{
> +    WCHAR *fullname, *p;
> +    char *unixpath;
> +    int namelen = 1;
> +
> +    if (path)
> +        namelen += strlen(path) + 1;
> +    if (file)
> +        namelen += strlen(file);
> +
> +    p = fullname = HeapAlloc(GetProcessHeap(), 0, namelen * sizeof(WCHAR));
> +    ZeroMemory(fullname, namelen * sizeof(WCHAR));

You should check for allocation failure. memset is preferred over ZeroMemory.