[pulseaudio-discuss] [PATCH] introspect: Fix ABI break introduced by b98a2e1

[Tanu Kaskinen]

On Mon, 2013-11-04 at 19:41 +0200, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz at intel.com>
> 
> The size of pa_card_profile_info cannot change even if it just a field
> appended to end because each entry is appended to a contiguous memory
> and accessed by offset this may lead clients to access invalid data.
> 
> To fix a new struct called pa_card_profile_info2 is introduced and shall
> be used for now on while pa_card_profile_info shall be considered
> deprecated but it is still mantained for backward compatibility.
> 
> A new field called profiles2 is introduced to pa_card_info, this new field
> is an array of pointers to pa_card_profile_info2 so it should be possible
> to append new fields to the end of the pa_card_profile_info2 without
> breaking binary compatibility as the entries are not accessed by offset.
> ---
>  src/pulse/introspect.c | 73 ++++++++++++++++++++++++++++++++++----------------
>  src/pulse/introspect.h | 26 +++++++++++++-----
>  src/utils/pactl.c      |  8 +++---
>  3 files changed, 73 insertions(+), 34 deletions(-)

Thanks! Patch applied. I fixed a couple of small issues pointed out
below.

> +static int fill_card_profile_info(pa_context *context, pa_tagstruct* t, pa_card_info* i) {
> +    uint32_t j;
> +
> +    i->profiles = pa_xnew0(pa_card_profile_info, i->n_profiles+1);
> +    i->profiles2 = pa_xnew0(pa_card_profile_info2*, i->n_profiles+1);
> +
> +    for (j = 0; j < i->n_profiles; j++) {
> +        if (pa_tagstruct_gets(t, &i->profiles[j].name) < 0 ||
> +            pa_tagstruct_gets(t, &i->profiles[j].description) < 0 ||
> +            pa_tagstruct_getu32(t, &i->profiles[j].n_sinks) < 0 ||
> +            pa_tagstruct_getu32(t, &i->profiles[j].n_sources) < 0 ||
> +            pa_tagstruct_getu32(t, &i->profiles[j].priority) < 0)
> +                return -PA_ERR_PROTOCOL;
> +
> +        i->profiles2[j] = pa_xnew0(pa_card_profile_info2, 1);
> +        i->profiles2[j]->name = i->profiles[j].name;
> +        i->profiles2[j]->description = i->profiles[j].description;
> +        i->profiles2[j]->n_sinks = i->profiles[j].n_sinks;
> +        i->profiles2[j]->n_sources = i->profiles[j].n_sources;
> +        i->profiles2[j]->priority = i->profiles[j].priority;
> +
> +        if (context->version >= 29) {
> +            uint32_t av;
> +
> +            if (pa_tagstruct_getu32(t, &av) < 0)
> +                return -PA_ERR_PROTOCOL;
> +
> +            i->profiles2[j]->available = av;
> +        }

The available flag should be initialized to 1 if the protocol version is
less than 29.

> @@ -484,11 +494,13 @@ typedef struct pa_card_info {
>      uint32_t owner_module;               /**< Index of the owning module, or PA_INVALID_INDEX. */
>      const char *driver;                  /**< Driver name */
>      uint32_t n_profiles;                 /**< Number of entries in profile array */
> -    pa_card_profile_info* profiles;      /**< Array of available profile, or NULL. Array is terminated by an entry with name set to NULL. Number of entries is stored in n_profiles. */
> -    pa_card_profile_info* active_profile; /**< Pointer to active profile in the array, or NULL. */
> +    pa_card_profile_info* profiles;      /**< \deprecated Superseded by profiles2 */
> +    pa_card_profile_info* active_profile; /**< \deprecated Superseded by active_profiles2 */

Typo in the comment: should be active_profile2, not active_profiles2.

-- 
Tanu