On Tue, 2012-02-07 at 10:26 +0100, David Henningsson wrote:
> On 02/07/2012 09:20 AM, Arun Raghavan wrote:
> > Since a given property can be single-valued, an array or (in the case of
> > ints) a range, clients need an API to figure out what type of value a
> > property holds. This adds such an API. The actual property type
> > enumeration is kept in the PA_PROP_* namespace and not the
> > PA_FORMAT_INFO* namespace so that it can later be reused for properties
> > generically if required.
> > ---
> > src/map-file | 1 +
> > src/pulse/format.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
> > src/pulse/format.h | 24 +++++++++++++++
> > src/tests/format-test.c | 5 +++
> > 4 files changed, 104 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/map-file b/src/map-file
> > index ee3d31d..5730fdf 100644
> > --- a/src/map-file
> > +++ b/src/map-file
> > @@ -168,6 +168,7 @@ pa_format_info_free;
> > pa_format_info_free2;
> > pa_format_info_from_string;
> > pa_format_info_from_sample_spec;
> > +pa_format_info_get_prop_type;
> > pa_format_info_get_prop_int;
> > pa_format_info_get_prop_int_range;
> > pa_format_info_get_prop_int_array;
> > diff --git a/src/pulse/format.c b/src/pulse/format.c
> > index 6176846..aee3dfc 100644
> > --- a/src/pulse/format.c
> > +++ b/src/pulse/format.c
> > @@ -280,6 +280,80 @@ int pa_format_info_to_sample_spec_fake(pa_format_info *f, pa_sample_spec *ss) {
> > return 0;
> > }
> >
> > +pa_prop_type_t pa_format_info_get_prop_type(pa_format_info *f, const char *key) {
> > + const char *str;
> > + json_object *o, *o1;
> > + pa_prop_type_t type;
> > +
> > + pa_assert(f);
> > + pa_assert(key);
> > +
> > + str = pa_proplist_gets(f->plist, key);
> > + if (!str)
> > + return PA_PROP_TYPE_INVALID;
> > +
> > + o = json_tokener_parse(str);
> > + if (is_error(o))
> > + return PA_PROP_TYPE_INVALID;
> > +
> > + switch (json_object_get_type(o)) {
> > + case json_type_int:
> > + type = PA_PROP_TYPE_INT;
> > + break;
> > +
> > + case json_type_string:
> > + type = PA_PROP_TYPE_STRING;
> > + break;
> > +
> > + case json_type_array:
> > + if (json_object_array_length(o) == 0) {
> > + /* Unlikely, but let's account for this anyway. We need at
> > + * least one element to figure out the array type. */
> > + type = PA_PROP_TYPE_INVALID;
> > + break;
> > + }
> > +
> > + o1 = json_object_array_get_idx(o, 1);
> > +
> > + if (json_object_get_type(o1) == json_type_int)
> > + type = PA_PROP_TYPE_INT_ARRAY;
> > + else if (json_object_get_type(o1) == json_type_string)
> > + type = PA_PROP_TYPE_STRING_ARRAY;
> > + else
> > + type = PA_PROP_TYPE_INVALID;
> > +
> > + json_object_put(o1);
> > + break;
> > +
> > + case json_type_object:
> > + /* We actually know at this point that it's a int range, but let's
> > + * confirm. */
> > + o1 = json_object_object_get(o, PA_JSON_MIN_KEY);
> > + if (!o1) {
> > + type = PA_PROP_TYPE_INVALID;
> > + break;
> > + }
> > + json_object_put(o1);
> > +
> > + o1 = json_object_object_get(o, PA_JSON_MIN_KEY);
>
> Shouldn't this be PA_JSON_MAX_KEY - and if it should, how come that the
> test succeed anyway? (Assuming you have actually tested that it does
> before submitting?)
The test succeeds because we're testing the same thing twice in this
code, so it's true both times. Crafting a case that causes this test to
fail would be quite painful (the actual condition for the range case. as
I've documented, is more out of paranoia than actual necessity).
-- Arun
