[Spice-devel] [PATCH v3 1/3] vdagent: move file xfer initialization to a function

[Christophe Fergeau]

On Thu, May 18, 2017 at 12:21:55PM +0200, Victor Toso wrote:
> From: Victor Toso <me at victortoso.com>
> 
> This patch creates two functions:
> - xfer_get_download_directory()
> - vdagent_init_file_xfer()
> 
> The logic should be similar as it was before this patch, taking in
> consideration the global variables fx_open_dir and fx_dir which are
> set from command line.

Fwiw, this is not true as this removes handling of
"xdg-download"/"xdg-desktop" from the command line. To prevent confusion
in the future/make a revert easier if needed/..., I would split this
removal in a separate patch (or handle these magic values in
xfer_get_download_directory())

> 
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
>  src/vdagent/vdagent.c | 73 +++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 53 insertions(+), 20 deletions(-)
> 
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index 9900303..03f8021 100644
> --- a/src/vdagent/vdagent.c
> +++ b/src/vdagent/vdagent.c
> @@ -25,6 +25,7 @@
>  
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <stdbool.h>
>  #include <string.h>
>  #include <syslog.h>
>  #include <unistd.h>
> @@ -55,6 +56,56 @@ static struct udscs_connection *client = NULL;
>  static int quit = 0;
>  static int version_mismatch = 0;
>  
> +/**
> + * xfer_get_download_directory
> + *
> + * Return path were file-transfer will save the files. Should be freed with
> + * g_free().
> + **/
> +static const gchar *xfer_get_download_directory(void)
> +{
> +    if (fx_dir != NULL)
> +        return fx_dir;
> +
> +    if (vdagent_x11_has_icons_on_desktop(x11))
> +        return g_get_user_special_dir(G_USER_DIRECTORY_DESKTOP);
> +
> +    return g_get_user_special_dir(G_USER_DIRECTORY_DOWNLOAD);
> +}
> +
> +/**
> + * vdagent_init_file_xfer
> + *
> + * Initialize handler for file xfer and returns true if vdagent_file_xfers is
> + * not NULL.
> + **/
> +static bool vdagent_init_file_xfer(void)
> +{
> +    bool xfer_open_dir;
> +    const gchar *xfer_dir;
> +
> +    if (vdagent_file_xfers != NULL) {
> +        syslog(LOG_WARNING, "File-xfer already initialized");

Does this deserves a syslog warning?

> +        return true;
> +    }
> +
> +    xfer_dir = xfer_get_download_directory();
> +    if (xfer_dir == NULL) {
> +        syslog(LOG_WARNING,
> +               "warning could not get file xfer save dir, "
> +               "file transfers will be disabled");
> +        vdagent_file_xfers = NULL;
> +        return false;
> +    }
> +
> +    /* Open directory either if requested or target path is not xdg-desktop */
> +    xfer_open_dir = fx_open_dir != -1 || !vdagent_x11_has_icons_on_desktop(x11);

This is hard to parse, and I'm not sure it's correct when '-o 0' is
used.

    if (fx_open_dir != -1) {
        xfer_open_dir = fx_open_dir;
    } else {
        xfer_open_dir = !vdagent_x11_has_icons_on_desktop(x11);
    }

should work.

Apart from this,
Acked-by: Christophe Fergeau <cfergeau at redhat.com>

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170529/8e54fe56/attachment-0001.sig>