[pulseaudio-discuss] [PATCH 2/2] context: add cookie loaders + setter

[Tanu Kaskinen]

On Tue, 2013-08-13 at 11:41 +0200, Alexander Couzens wrote:
> I'm not sure if these 3 seperate calls for cookie loading are better than creating a pa_context_get_conf() call and
> operating on it.

I think it's better to keep pa_client_conf as a private interface.

> 
> Signed-off-by: Alexander Couzens <lynxis at fe80.eu>
> ---
>  src/pulse/context.c | 15 +++++++++++++++
>  src/pulse/context.h |  9 +++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/src/pulse/context.c b/src/pulse/context.c
> index 1ba2672..654284e 100644
> --- a/src/pulse/context.c
> +++ b/src/pulse/context.c
> @@ -1448,3 +1448,18 @@ size_t pa_context_get_tile_size(pa_context *c, const pa_sample_spec *ss) {
>      mbs = PA_ROUND_DOWN(pa_mempool_block_size_max(c->mempool), fs);
>      return PA_MAX(mbs, fs);
>  }
> +
> +int pa_context_set_cookie(pa_context *c, uint8_t *cookie, size_t cookie_size) {

The pointers should be checked with pa_assert(). And there should be
some sanity checks:

    PA_CHECK_VALIDITY(c, !pa_detect_fork(), PA_ERR_FORKED);
    PA_CHECK_VALIDITY(c, c->state == PA_CONTEXT_UNCONNECTED, PA_ERR_BADSTATE);

(The same goes for the other functions.)

> +    if(cookie_size != PA_NATIVE_COOKIE_LENGTH)

Missing space after "if".

> +        return -PA_ERR_INVALID;
> +    memcpy(c->conf->cookie, cookie, cookie_size);

c->conf->cookie_valid should be set to true. Or actually, there should
be pa_client_conf_set_cookie() that sets the cookie_valid field -
modifying the pa_client_conf state should be done from client-conf.c
only.

> +    return 0;
> +}
> +
> +int pa_context_load_cookie_from_hex(pa_context *c, const char *cookie_as_hex) {
> +    return pa_client_conf_load_cookie_from_hex(c->conf, cookie_as_hex);
> +}
> +
> +int pa_context_load_cookie_file(pa_context *c, const char *cookie_file_path) {
> +    return pa_client_conf_load_cookie_from_file(c->conf, cookie_file_path);
> +}
> diff --git a/src/pulse/context.h b/src/pulse/context.h
> index 6e615f6..20fa1ff 100644
> --- a/src/pulse/context.h
> +++ b/src/pulse/context.h
> @@ -280,6 +280,15 @@ void pa_context_rttime_restart(pa_context *c, pa_time_event *e, pa_usec_t usec);
>   * pa_stream_get_sample_spec(ss)); \since 0.9.20 */
>  size_t pa_context_get_tile_size(pa_context *c, const pa_sample_spec *ss);
>  
> +/** Set the authentication cookie */

The documentation is missing a \since 5.0 tag, and explanation about
what the cookie size should be.

We need to expose the cookie size to clients somehow. A simple #define
should be good enough. The problem with a #define in the public API is
that it can't be changed without breaking compatibility, but I don't
think we can change the cookie size anyway without breaking
compatibility between old server and new libpulse or vice versa.

-- 
Tanu