modargs: Ensure modargs can be accessed in their raw form.

When dealing with proplists passed as modargs, we need the unescaped form
in order to properly deal with quotes (ticks + double quotes). As the previous
code always called pa_unescape() before adding it into the modarg hashmap, this
was impossible.

This modification simply stores two proplists. If the unescaped value
is different from the raw value, we also keep the raw form.

When parsing proplist arguments, we use this raw form and do the unescaping
ourselves when processing it.

This changes the current behaviour which required you to double escape
proplists arguments. This double escape mechanism did allow you to mix
and match what types of quotes you used to delimit the individial
proplist values, but it made the actual data much harder to pass in.

This approach has the drawback that you cannot mix and match the quotes
you use, but this is a very minor issue and IMO pales in comparison to
the general clarity gained.

See the discussion on the mailing list for more background:
 http://lists.freedesktop.org/archives/pulseaudio-discuss/2011-September/011220.html
This commit is contained in:
Colin Guthrie 2011-09-06 11:35:33 +01:00
parent e93b32744c
commit 3542112888
2 changed files with 66 additions and 36 deletions

View file

@ -36,18 +36,26 @@
#include "modargs.h" #include "modargs.h"
struct pa_modargs {
pa_hashmap *raw;
pa_hashmap *unescaped;
};
struct entry { struct entry {
char *key, *value; char *key, *value;
}; };
static int add_key_value(pa_hashmap *map, char *key, char *value, const char* const valid_keys[]) { static int add_key_value(pa_modargs *ma, char *key, char *value, const char* const valid_keys[]) {
struct entry *e; struct entry *e;
char *raw;
pa_assert(map); pa_assert(ma);
pa_assert(ma->raw);
pa_assert(ma->unescaped);
pa_assert(key); pa_assert(key);
pa_assert(value); pa_assert(value);
if (pa_hashmap_get(map, key)) { if (pa_hashmap_get(ma->unescaped, key)) {
pa_xfree(key); pa_xfree(key);
pa_xfree(value); pa_xfree(value);
return -1; return -1;
@ -66,10 +74,21 @@ static int add_key_value(pa_hashmap *map, char *key, char *value, const char* co
} }
} }
raw = pa_xstrdup(value);
e = pa_xnew(struct entry, 1); e = pa_xnew(struct entry, 1);
e->key = key; e->key = key;
e->value = value; e->value = pa_unescape(value);
pa_hashmap_put(map, key, e); pa_hashmap_put(ma->unescaped, key, e);
if (pa_streq(raw, value))
pa_xfree(raw);
else {
e = pa_xnew(struct entry, 1);
e->key = pa_xstrdup(key);
e->value = raw;
pa_hashmap_put(ma->raw, key, e);
}
return 0; return 0;
} }
@ -89,12 +108,13 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
const char *p, *key = NULL, *value = NULL; const char *p, *key = NULL, *value = NULL;
size_t key_len = 0, value_len = 0; size_t key_len = 0, value_len = 0;
pa_hashmap *map; pa_modargs *ma = pa_xnew(pa_modargs, 1);
map = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func); ma->raw = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
ma->unescaped = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
if (!args) if (!args)
return (pa_modargs*) map; return ma;
state = WHITESPACE; state = WHITESPACE;
@ -130,7 +150,7 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
value = p+1; value = p+1;
value_len = 0; value_len = 0;
} else if (isspace(*p)) { } else if (isspace(*p)) {
if (add_key_value(map, if (add_key_value(ma,
pa_xstrndup(key, key_len), pa_xstrndup(key, key_len),
pa_xstrdup(""), pa_xstrdup(""),
valid_keys) < 0) valid_keys) < 0)
@ -149,9 +169,9 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
case VALUE_SIMPLE: case VALUE_SIMPLE:
if (isspace(*p)) { if (isspace(*p)) {
if (add_key_value(map, if (add_key_value(ma,
pa_xstrndup(key, key_len), pa_xstrndup(key, key_len),
pa_unescape(pa_xstrndup(value, value_len)), pa_xstrndup(value, value_len),
valid_keys) < 0) valid_keys) < 0)
goto fail; goto fail;
state = WHITESPACE; state = WHITESPACE;
@ -169,9 +189,9 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
case VALUE_DOUBLE_QUOTES: case VALUE_DOUBLE_QUOTES:
if (*p == '"') { if (*p == '"') {
if (add_key_value(map, if (add_key_value(ma,
pa_xstrndup(key, key_len), pa_xstrndup(key, key_len),
pa_unescape(pa_xstrndup(value, value_len)), pa_xstrndup(value, value_len),
valid_keys) < 0) valid_keys) < 0)
goto fail; goto fail;
state = WHITESPACE; state = WHITESPACE;
@ -189,9 +209,9 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
case VALUE_TICKS: case VALUE_TICKS:
if (*p == '\'') { if (*p == '\'') {
if (add_key_value(map, if (add_key_value(ma,
pa_xstrndup(key, key_len), pa_xstrndup(key, key_len),
pa_unescape(pa_xstrndup(value, value_len)), pa_xstrndup(value, value_len),
valid_keys) < 0) valid_keys) < 0)
goto fail; goto fail;
state = WHITESPACE; state = WHITESPACE;
@ -210,19 +230,19 @@ pa_modargs *pa_modargs_new(const char *args, const char* const* valid_keys) {
} }
if (state == VALUE_START) { if (state == VALUE_START) {
if (add_key_value(map, pa_xstrndup(key, key_len), pa_xstrdup(""), valid_keys) < 0) if (add_key_value(ma, pa_xstrndup(key, key_len), pa_xstrdup(""), valid_keys) < 0)
goto fail; goto fail;
} else if (state == VALUE_SIMPLE) { } else if (state == VALUE_SIMPLE) {
if (add_key_value(map, pa_xstrndup(key, key_len), pa_xstrdup(value), valid_keys) < 0) if (add_key_value(ma, pa_xstrndup(key, key_len), pa_xstrdup(value), valid_keys) < 0)
goto fail; goto fail;
} else if (state != WHITESPACE) } else if (state != WHITESPACE)
goto fail; goto fail;
return (pa_modargs*) map; return ma;
fail: fail:
pa_modargs_free((pa_modargs*) map); pa_modargs_free(ma);
return NULL; return NULL;
} }
@ -237,15 +257,33 @@ static void free_func(void *p, void*userdata) {
} }
void pa_modargs_free(pa_modargs*ma) { void pa_modargs_free(pa_modargs*ma) {
pa_hashmap *map = (pa_hashmap*) ma; pa_assert(ma);
pa_hashmap_free(map, free_func, NULL);
pa_hashmap_free(ma->raw, free_func, NULL);
pa_hashmap_free(ma->unescaped, free_func, NULL);
pa_xfree(ma);
} }
const char *pa_modargs_get_value(pa_modargs *ma, const char *key, const char *def) { const char *pa_modargs_get_value(pa_modargs *ma, const char *key, const char *def) {
pa_hashmap *map = (pa_hashmap*) ma;
struct entry*e; struct entry*e;
if (!(e = pa_hashmap_get(map, key))) pa_assert(ma);
pa_assert(key);
if (!(e = pa_hashmap_get(ma->unescaped, key)))
return def;
return e->value;
}
static const char *modargs_get_value_raw(pa_modargs *ma, const char *key, const char *def) {
struct entry*e;
pa_assert(ma);
pa_assert(key);
if (!(e = pa_hashmap_get(ma->raw, key)))
if (!(e = pa_hashmap_get(ma->unescaped, key)))
return def; return def;
return e->value; return e->value;
@ -254,8 +292,6 @@ const char *pa_modargs_get_value(pa_modargs *ma, const char *key, const char *de
int pa_modargs_get_value_u32(pa_modargs *ma, const char *key, uint32_t *value) { int pa_modargs_get_value_u32(pa_modargs *ma, const char *key, uint32_t *value) {
const char *v; const char *v;
pa_assert(ma);
pa_assert(key);
pa_assert(value); pa_assert(value);
if (!(v = pa_modargs_get_value(ma, key, NULL))) if (!(v = pa_modargs_get_value(ma, key, NULL)))
@ -270,8 +306,6 @@ int pa_modargs_get_value_u32(pa_modargs *ma, const char *key, uint32_t *value) {
int pa_modargs_get_value_s32(pa_modargs *ma, const char *key, int32_t *value) { int pa_modargs_get_value_s32(pa_modargs *ma, const char *key, int32_t *value) {
const char *v; const char *v;
pa_assert(ma);
pa_assert(key);
pa_assert(value); pa_assert(value);
if (!(v = pa_modargs_get_value(ma, key, NULL))) if (!(v = pa_modargs_get_value(ma, key, NULL)))
@ -287,8 +321,6 @@ int pa_modargs_get_value_boolean(pa_modargs *ma, const char *key, pa_bool_t *val
const char *v; const char *v;
int r; int r;
pa_assert(ma);
pa_assert(key);
pa_assert(value); pa_assert(value);
if (!(v = pa_modargs_get_value(ma, key, NULL))) if (!(v = pa_modargs_get_value(ma, key, NULL)))
@ -309,7 +341,6 @@ int pa_modargs_get_sample_spec(pa_modargs *ma, pa_sample_spec *rss) {
uint32_t channels; uint32_t channels;
pa_sample_spec ss; pa_sample_spec ss;
pa_assert(ma);
pa_assert(rss); pa_assert(rss);
ss = *rss; ss = *rss;
@ -341,7 +372,6 @@ int pa_modargs_get_channel_map(pa_modargs *ma, const char *name, pa_channel_map
pa_channel_map map; pa_channel_map map;
const char *cm; const char *cm;
pa_assert(ma);
pa_assert(rmap); pa_assert(rmap);
map = *rmap; map = *rmap;
@ -366,7 +396,6 @@ int pa_modargs_get_sample_spec_and_channel_map(
pa_sample_spec ss; pa_sample_spec ss;
pa_channel_map map; pa_channel_map map;
pa_assert(ma);
pa_assert(rss); pa_assert(rss);
pa_assert(rmap); pa_assert(rmap);
@ -400,7 +429,7 @@ int pa_modargs_get_proplist(pa_modargs *ma, const char *name, pa_proplist *p, pa
pa_assert(name); pa_assert(name);
pa_assert(p); pa_assert(p);
if (!(v = pa_modargs_get_value(ma, name, NULL))) if (!(v = modargs_get_value_raw(ma, name, NULL)))
return 0; return 0;
if (!(n = pa_proplist_from_string(v))) if (!(n = pa_proplist_from_string(v)))
@ -413,10 +442,11 @@ int pa_modargs_get_proplist(pa_modargs *ma, const char *name, pa_proplist *p, pa
} }
const char *pa_modargs_iterate(pa_modargs *ma, void **state) { const char *pa_modargs_iterate(pa_modargs *ma, void **state) {
pa_hashmap *map = (pa_hashmap*) ma;
struct entry *e; struct entry *e;
if (!(e = pa_hashmap_iterate(map, state, NULL))) pa_assert(ma);
if (!(e = pa_hashmap_iterate(ma->unescaped, state, NULL)))
return NULL; return NULL;
return e->key; return e->key;

View file

@ -81,7 +81,7 @@ int main(int argc, char*argv[]) {
printf("%s\n", v); printf("%s\n", v);
pa_xfree(v); pa_xfree(v);
pa_assert_se(ma = pa_modargs_new("foo='foobar=waldo foo2=\"lj\\\\\"dhflh\" foo3=\\'kjlskj\\\\\\'\\''", x)); pa_assert_se(ma = pa_modargs_new("foo='foobar=waldo foo2=\"lj\\\"dhflh\" foo3=\"kjlskj\\'\"'", x));
pa_assert_se(a = pa_proplist_new()); pa_assert_se(a = pa_proplist_new());
pa_assert_se(pa_modargs_get_proplist(ma, "foo", a, PA_UPDATE_REPLACE) >= 0); pa_assert_se(pa_modargs_get_proplist(ma, "foo", a, PA_UPDATE_REPLACE) >= 0);