[pulseaudio-discuss] [PATCH 1/2] client-conf: refactored cookie loaders

[Tanu Kaskinen]

Still some nitpicking about the commit message: please use the present
tense ("refactor" instead of "refactored").

BTW, here's a good blog post about commit messages that I saw recently,
recommended reading for everybody:
http://who-t.blogspot.de/2009/12/on-commit-messages.html

On Wed, 2013-08-21 at 13:18 +0200, Alexander Couzens wrote:
> Signed-off-by: Alexander Couzens <lynxis at fe80.eu>
> ---
>  src/pulse/client-conf-x11.c | 12 +-----------
>  src/pulse/client-conf.c     | 48 +++++++++++++++++++++++++++++++++++++++------
>  src/pulse/client-conf.h     | 10 ++++++++--
>  3 files changed, 51 insertions(+), 19 deletions(-)
> 
> diff --git a/src/pulse/client-conf-x11.c b/src/pulse/client-conf-x11.c
> index 99265c5..8d0c612 100644
> --- a/src/pulse/client-conf-x11.c
> +++ b/src/pulse/client-conf-x11.c
> @@ -91,20 +91,10 @@ int pa_client_conf_from_x11(pa_client_conf *c, const char *dname) {
>      }
>  
>      if (pa_x11_get_prop(xcb, screen, "PULSE_COOKIE", t, sizeof(t))) {
> -        uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
> -
> -        if (pa_parsehex(t, cookie, sizeof(cookie)) != sizeof(cookie)) {
> +        if (pa_client_conf_load_cookie_from_hex(c, t) < 0) {
>              pa_log(_("Failed to parse cookie data"));
>              goto finish;
>          }
> -
> -        pa_assert(sizeof(cookie) == sizeof(c->cookie));
> -        memcpy(c->cookie, cookie, sizeof(cookie));
> -
> -        c->cookie_valid = true;
> -
> -        pa_xfree(c->cookie_file);
> -        c->cookie_file = NULL;
>      }
>  
>      ret = 0;
> diff --git a/src/pulse/client-conf.c b/src/pulse/client-conf.c
> index 8301981..ee0271b 100644
> --- a/src/pulse/client-conf.c
> +++ b/src/pulse/client-conf.c
> @@ -66,6 +66,8 @@ static const pa_client_conf default_conf = {
>      .auto_connect_display = false
>  };
>  
> +static int parse_cookie_file(pa_client_conf* c);
> +
>  pa_client_conf *pa_client_conf_new(void) {
>      pa_client_conf *c = pa_xmemdup(&default_conf, sizeof(default_conf));
>  
> @@ -130,7 +132,7 @@ int pa_client_conf_load(pa_client_conf *c, const char *filename) {
>      r = f ? pa_config_parse(fn, f, table, NULL, NULL) : 0;
>  
>      if (!r)
> -        r = pa_client_conf_load_cookie(c);
> +        r = parse_cookie_file(c);
>  
>  finish:
>      pa_xfree(fn);
> @@ -168,16 +170,13 @@ int pa_client_conf_env(pa_client_conf *c) {
>      }
>  
>      if ((e = getenv(ENV_COOKIE_FILE))) {
> -        pa_xfree(c->cookie_file);
> -        c->cookie_file = pa_xstrdup(e);
> -
> -        return pa_client_conf_load_cookie(c);
> +        return pa_client_conf_load_cookie_from_file(c, e);
>      }
>  
>      return 0;
>  }
>  
> -int pa_client_conf_load_cookie(pa_client_conf* c) {
> +static int parse_cookie_file(pa_client_conf* c) {
>      int k;
>  
>      pa_assert(c);
> @@ -203,3 +202,40 @@ int pa_client_conf_load_cookie(pa_client_conf* c) {
>      c->cookie_valid = true;
>      return 0;
>  }
> +
> +int pa_client_conf_load_cookie_from_hex(pa_client_conf* c, const char *cookie_in_hex) {
> +    uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
> +
> +    pa_assert(c);
> +    pa_assert(cookie_in_hex);
> +
> +    if (pa_parsehex(cookie_in_hex, cookie, sizeof(cookie)) != sizeof(cookie)) {
> +        pa_log(_("Failed to parse cookie data"));
> +        return -PA_ERR_INVALID;
> +    }
> +
> +    pa_xfree(c->cookie_file);
> +    c->cookie_file = NULL;
> +
> +    return pa_client_conf_set_cookie(c, cookie, PA_NATIVE_COOKIE_LENGTH);
> +}
> +
> +int pa_client_conf_load_cookie_from_file(pa_client_conf *c, const char *cookie_file_path) {
> +    pa_assert(c);
> +    pa_assert(cookie_file_path);
> +
> +    pa_xfree(c->cookie_file);
> +    c->cookie_file = pa_xstrdup(cookie_file_path);
> +    return parse_cookie_file(c);
> +}
> +
> +int pa_client_conf_set_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_size) {
> +    pa_assert(c);
> +    pa_assert(cookie);
> +
> +    if (cookie_size != PA_NATIVE_COOKIE_LENGTH)
> +        return -PA_ERR_INVALID;
> +    memcpy(c->cookie, cookie, cookie_size);
> +    c->cookie_valid = true;
> +    return 0;
> +}
> diff --git a/src/pulse/client-conf.h b/src/pulse/client-conf.h
> index 9c509f7..87d42d8 100644
> --- a/src/pulse/client-conf.h
> +++ b/src/pulse/client-conf.h
> @@ -48,7 +48,13 @@ int pa_client_conf_load(pa_client_conf *c, const char *filename);
>     process, overwriting the current settings in *c. */
>  int pa_client_conf_env(pa_client_conf *c);
>  

It looks like the patch got truncated.

-- 
Tanu