[alsa-devel] [PATCH - UCM 1/2] control: enable octal and hexadecimal parse
From: "Lu, Han" han.lu@intel.com
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/src/control/ctlparse.c b/src/control/ctlparse.c index 978977d..acaf734 100644 --- a/src/control/ctlparse.c +++ b/src/control/ctlparse.c @@ -59,10 +59,10 @@ static long get_integer(const char **ptr, long min, long max) goto out;
s = p; - val = strtol(s, &p, 10); + val = strtol(s, &p, 0); if (*p == '.') { p++; - strtol(p, &p, 10); + strtol(p, &p, 0); } if (*p == '%') { val = (long)convert_prange1(strtod(s, NULL), min, max); @@ -87,10 +87,10 @@ static long long get_integer64(const char **ptr, long long min, long long max) goto out;
s = p; - val = strtol(s, &p, 10); + val = strtol(s, &p, 0); if (*p == '.') { p++; - strtol(p, &p, 10); + strtol(p, &p, 0); } if (*p == '%') { val = (long long)convert_prange1(strtod(s, NULL), min, max);
From: "Lu, Han" han.lu@intel.com
with cset command, UCM set kcontrol parameters directly: cset "name='<KCONTROL_NAME>' 1,2<,3,...>" This patch enables UCM to set kcontrol with parameters from configure file: bcsetf "name='<KCONTROL_NAME>' <path/to/file>" where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/src/ucm/main.c b/src/ucm/main.c index 37ae4c8..1496b22 100644 --- a/src/ucm/main.c +++ b/src/ucm/main.c @@ -160,11 +160,45 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr, return 0; }
+static int binary_file_parse(snd_ctl_elem_value_t *dst, + const char *filepath) +{ + int err = 0; + FILE *in; + long len; + char *res; + unsigned int idx; + + in = fopen(filepath, "r"); + if (!in) { + err = -errno; + goto __fail; + } + fseek(in, 0L, SEEK_END); + len = ftell(in); + rewind(in); + if (len > 512) + len = 512; + res = calloc(1, (size_t)len); + if (res == NULL) { + err = -ENOMEM; + goto __fail_nomem; + } + fread(res, (size_t)len, 1, in); + for (idx = 0; idx < len; idx++) + snd_ctl_elem_value_set_byte(dst, idx, *(res + idx)); + free(res); + __fail_nomem: + fclose(in); + __fail: + return err; +} + extern int __snd_ctl_ascii_elem_id_parse(snd_ctl_elem_id_t *dst, const char *str, const char **ret_ptr);
-static int execute_cset(snd_ctl_t *ctl, const char *cset) +static int execute_cset(snd_ctl_t *ctl, const char *cset, int isbin) { const char *pos; int err; @@ -194,7 +228,10 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset) err = snd_ctl_elem_info(ctl, info); if (err < 0) goto __fail; - err = snd_ctl_ascii_value_parse(ctl, value, info, pos); + if (isbin) + err = binary_file_parse(value, pos); + else + err = snd_ctl_ascii_value_parse(ctl, value, info, pos); if (err < 0) goto __fail; err = snd_ctl_elem_write(ctl, value); @@ -239,6 +276,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail_nomem; break; case SEQUENCE_ELEMENT_TYPE_CSET: + case SEQUENCE_ELEMENT_TYPE_BCSETF: if (cdev == NULL) { const char *cdev1 = NULL, *cdev2 = NULL; err = get_value3(&cdev1, "PlaybackCTL", @@ -274,7 +312,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail; } } - err = execute_cset(ctl, s->data.cset); + err = execute_cset(ctl, s->data.cset, s->isbin); if (err < 0) { uc_error("unable to execute cset '%s'\n", s->data.cset); goto __fail; diff --git a/src/ucm/parser.c b/src/ucm/parser.c index d7517f6..686c883 100644 --- a/src/ucm/parser.c +++ b/src/ucm/parser.c @@ -306,6 +306,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, continue; }
+ if (strcmp(cmd, "bcsetf") == 0) { + curr->type = SEQUENCE_ELEMENT_TYPE_BCSETF; + curr->isbin = 1; + err = parse_string(n, &curr->data.cset); + if (err < 0) { + uc_error("error: bcsetf requires a string!"); + return err; + } + continue; + } + if (strcmp(cmd, "usleep") == 0) { curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP; err = snd_config_get_integer(n, &curr->data.sleep); diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h index 87f14a2..80d7335 100644 --- a/src/ucm/ucm_local.h +++ b/src/ucm/ucm_local.h @@ -47,6 +47,7 @@ #define SEQUENCE_ELEMENT_TYPE_CSET 2 #define SEQUENCE_ELEMENT_TYPE_SLEEP 3 #define SEQUENCE_ELEMENT_TYPE_EXEC 4 +#define SEQUENCE_ELEMENT_TYPE_BCSETF 5
struct ucm_value { struct list_head list; @@ -63,6 +64,7 @@ struct sequence_element { char *cset; char *exec; } data; + int isbin; /* Indicate cset is binary array or ascii array */ };
/*
At Tue, 13 Jan 2015 11:00:39 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
with cset command, UCM set kcontrol parameters directly: cset "name='<KCONTROL_NAME>' 1,2<,3,...>" This patch enables UCM to set kcontrol with parameters from configure file: bcsetf "name='<KCONTROL_NAME>' <path/to/file>" where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).
Why binary? It's not portable. You can't carry it to a different architecture.
Takashi
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/src/ucm/main.c b/src/ucm/main.c index 37ae4c8..1496b22 100644 --- a/src/ucm/main.c +++ b/src/ucm/main.c @@ -160,11 +160,45 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr, return 0; }
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
const char *filepath)+{
- int err = 0;
- FILE *in;
- long len;
- char *res;
- unsigned int idx;
- in = fopen(filepath, "r");
- if (!in) {
err = -errno;goto __fail;- }
- fseek(in, 0L, SEEK_END);
- len = ftell(in);
- rewind(in);
- if (len > 512)
len = 512;- res = calloc(1, (size_t)len);
- if (res == NULL) {
err = -ENOMEM;goto __fail_nomem;- }
- fread(res, (size_t)len, 1, in);
- for (idx = 0; idx < len; idx++)
snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));- free(res);
__fail_nomem:- fclose(in);
__fail:- return err;
+}
extern int __snd_ctl_ascii_elem_id_parse(snd_ctl_elem_id_t *dst, const char *str, const char **ret_ptr);
-static int execute_cset(snd_ctl_t *ctl, const char *cset) +static int execute_cset(snd_ctl_t *ctl, const char *cset, int isbin) { const char *pos; int err; @@ -194,7 +228,10 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset) err = snd_ctl_elem_info(ctl, info); if (err < 0) goto __fail;
- err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
- if (isbin)
err = binary_file_parse(value, pos);- else
err = snd_ctl_ascii_value_parse(ctl, value, info, pos);@@ -239,6 +276,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail_nomem; break; case SEQUENCE_ELEMENT_TYPE_CSET:
case SEQUENCE_ELEMENT_TYPE_BCSETF: if (cdev == NULL) { const char *cdev1 = NULL, *cdev2 = NULL; err = get_value3(&cdev1, "PlaybackCTL",@@ -274,7 +312,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail; } }
err = execute_cset(ctl, s->data.cset);
err = execute_cset(ctl, s->data.cset, s->isbin); if (err < 0) { uc_error("unable to execute cset '%s'\n", s->data.cset); goto __fail;diff --git a/src/ucm/parser.c b/src/ucm/parser.c index d7517f6..686c883 100644 --- a/src/ucm/parser.c +++ b/src/ucm/parser.c @@ -306,6 +306,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, continue; }
if (strcmp(cmd, "bcsetf") == 0) {curr->type = SEQUENCE_ELEMENT_TYPE_BCSETF;curr->isbin = 1;err = parse_string(n, &curr->data.cset);if (err < 0) {uc_error("error: bcsetf requires a string!");return err;}continue;}- if (strcmp(cmd, "usleep") == 0) { curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP; err = snd_config_get_integer(n, &curr->data.sleep);
diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h index 87f14a2..80d7335 100644 --- a/src/ucm/ucm_local.h +++ b/src/ucm/ucm_local.h @@ -47,6 +47,7 @@ #define SEQUENCE_ELEMENT_TYPE_CSET 2 #define SEQUENCE_ELEMENT_TYPE_SLEEP 3 #define SEQUENCE_ELEMENT_TYPE_EXEC 4 +#define SEQUENCE_ELEMENT_TYPE_BCSETF 5
struct ucm_value { struct list_head list; @@ -63,6 +64,7 @@ struct sequence_element { char *cset; char *exec; } data;
- int isbin; /* Indicate cset is binary array or ascii array */
};
/*
2.1.0
On Tue, 2015-01-13 at 17:52 +0100, Takashi Iwai wrote:
At Tue, 13 Jan 2015 11:00:39 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
with cset command, UCM set kcontrol parameters directly: cset "name='<KCONTROL_NAME>' 1,2<,3,...>" This patch enables UCM to set kcontrol with parameters from configure file: bcsetf "name='<KCONTROL_NAME>' <path/to/file>" where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).
Why binary? It's not portable. You can't carry it to a different architecture.
The intention here is that the binary data is not meant for the host but for audio DSPs so it's just passed by UCM/ALSA as raw data.
We do have some DSP processing algos that have tuning tools where the tool will generate a binary file of coefficients (depending on various physical device properties etc). These coefficient files can then be uploaded to the DSP by UCM on use case change.
Liam
Takashi
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/src/ucm/main.c b/src/ucm/main.c index 37ae4c8..1496b22 100644 --- a/src/ucm/main.c +++ b/src/ucm/main.c @@ -160,11 +160,45 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr, return 0; }
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
const char *filepath)+{
- int err = 0;
- FILE *in;
- long len;
- char *res;
- unsigned int idx;
- in = fopen(filepath, "r");
- if (!in) {
err = -errno;goto __fail;- }
- fseek(in, 0L, SEEK_END);
- len = ftell(in);
- rewind(in);
- if (len > 512)
len = 512;- res = calloc(1, (size_t)len);
- if (res == NULL) {
err = -ENOMEM;goto __fail_nomem;- }
- fread(res, (size_t)len, 1, in);
- for (idx = 0; idx < len; idx++)
snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));- free(res);
__fail_nomem:- fclose(in);
__fail:- return err;
+}
extern int __snd_ctl_ascii_elem_id_parse(snd_ctl_elem_id_t *dst, const char *str, const char **ret_ptr);
-static int execute_cset(snd_ctl_t *ctl, const char *cset) +static int execute_cset(snd_ctl_t *ctl, const char *cset, int isbin) { const char *pos; int err; @@ -194,7 +228,10 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset) err = snd_ctl_elem_info(ctl, info); if (err < 0) goto __fail;
- err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
- if (isbin)
err = binary_file_parse(value, pos);- else
err = snd_ctl_ascii_value_parse(ctl, value, info, pos);@@ -239,6 +276,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail_nomem; break; case SEQUENCE_ELEMENT_TYPE_CSET:
case SEQUENCE_ELEMENT_TYPE_BCSETF: if (cdev == NULL) { const char *cdev1 = NULL, *cdev2 = NULL; err = get_value3(&cdev1, "PlaybackCTL",@@ -274,7 +312,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail; } }
err = execute_cset(ctl, s->data.cset);
err = execute_cset(ctl, s->data.cset, s->isbin); if (err < 0) { uc_error("unable to execute cset '%s'\n", s->data.cset); goto __fail;diff --git a/src/ucm/parser.c b/src/ucm/parser.c index d7517f6..686c883 100644 --- a/src/ucm/parser.c +++ b/src/ucm/parser.c @@ -306,6 +306,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, continue; }
if (strcmp(cmd, "bcsetf") == 0) {curr->type = SEQUENCE_ELEMENT_TYPE_BCSETF;curr->isbin = 1;err = parse_string(n, &curr->data.cset);if (err < 0) {uc_error("error: bcsetf requires a string!");return err;}continue;}- if (strcmp(cmd, "usleep") == 0) { curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP; err = snd_config_get_integer(n, &curr->data.sleep);
diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h index 87f14a2..80d7335 100644 --- a/src/ucm/ucm_local.h +++ b/src/ucm/ucm_local.h @@ -47,6 +47,7 @@ #define SEQUENCE_ELEMENT_TYPE_CSET 2 #define SEQUENCE_ELEMENT_TYPE_SLEEP 3 #define SEQUENCE_ELEMENT_TYPE_EXEC 4 +#define SEQUENCE_ELEMENT_TYPE_BCSETF 5
struct ucm_value { struct list_head list; @@ -63,6 +64,7 @@ struct sequence_element { char *cset; char *exec; } data;
- int isbin; /* Indicate cset is binary array or ascii array */
};
/*
2.1.0
At Tue, 13 Jan 2015 18:26:18 +0000, Liam Girdwood wrote:
On Tue, 2015-01-13 at 17:52 +0100, Takashi Iwai wrote:
At Tue, 13 Jan 2015 11:00:39 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
with cset command, UCM set kcontrol parameters directly: cset "name='<KCONTROL_NAME>' 1,2<,3,...>" This patch enables UCM to set kcontrol with parameters from configure file: bcsetf "name='<KCONTROL_NAME>' <path/to/file>" where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).
Why binary? It's not portable. You can't carry it to a different architecture.
The intention here is that the binary data is not meant for the host but for audio DSPs so it's just passed by UCM/ALSA as raw data.
In that case, we should limit to certain element data types. Otherwise people would abuse it for passing data even to integer or enum ctls.
And of course it'd be better to clarify the reason in the patch description :)
BTW, I'm still not so convinced by bcsetf... Can't it be more verbose or readable?
thanks,
Takashi
We do have some DSP processing algos that have tuning tools where the tool will generate a binary file of coefficients (depending on various physical device properties etc). These coefficient files can then be uploaded to the DSP by UCM on use case change.
Liam
Takashi
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/src/ucm/main.c b/src/ucm/main.c index 37ae4c8..1496b22 100644 --- a/src/ucm/main.c +++ b/src/ucm/main.c @@ -160,11 +160,45 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr, return 0; }
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
const char *filepath)+{
- int err = 0;
- FILE *in;
- long len;
- char *res;
- unsigned int idx;
- in = fopen(filepath, "r");
- if (!in) {
err = -errno;goto __fail;- }
- fseek(in, 0L, SEEK_END);
- len = ftell(in);
- rewind(in);
- if (len > 512)
len = 512;- res = calloc(1, (size_t)len);
- if (res == NULL) {
err = -ENOMEM;goto __fail_nomem;- }
- fread(res, (size_t)len, 1, in);
- for (idx = 0; idx < len; idx++)
snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));- free(res);
__fail_nomem:- fclose(in);
__fail:- return err;
+}
extern int __snd_ctl_ascii_elem_id_parse(snd_ctl_elem_id_t *dst, const char *str, const char **ret_ptr);
-static int execute_cset(snd_ctl_t *ctl, const char *cset) +static int execute_cset(snd_ctl_t *ctl, const char *cset, int isbin) { const char *pos; int err; @@ -194,7 +228,10 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset) err = snd_ctl_elem_info(ctl, info); if (err < 0) goto __fail;
- err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
- if (isbin)
err = binary_file_parse(value, pos);- else
err = snd_ctl_ascii_value_parse(ctl, value, info, pos);@@ -239,6 +276,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail_nomem; break; case SEQUENCE_ELEMENT_TYPE_CSET:
case SEQUENCE_ELEMENT_TYPE_BCSETF: if (cdev == NULL) { const char *cdev1 = NULL, *cdev2 = NULL; err = get_value3(&cdev1, "PlaybackCTL",@@ -274,7 +312,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr, goto __fail; } }
err = execute_cset(ctl, s->data.cset);
err = execute_cset(ctl, s->data.cset, s->isbin); if (err < 0) { uc_error("unable to execute cset '%s'\n", s->data.cset); goto __fail;diff --git a/src/ucm/parser.c b/src/ucm/parser.c index d7517f6..686c883 100644 --- a/src/ucm/parser.c +++ b/src/ucm/parser.c @@ -306,6 +306,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, continue; }
if (strcmp(cmd, "bcsetf") == 0) {curr->type = SEQUENCE_ELEMENT_TYPE_BCSETF;curr->isbin = 1;err = parse_string(n, &curr->data.cset);if (err < 0) {uc_error("error: bcsetf requires a string!");return err;}continue;}- if (strcmp(cmd, "usleep") == 0) { curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP; err = snd_config_get_integer(n, &curr->data.sleep);
diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h index 87f14a2..80d7335 100644 --- a/src/ucm/ucm_local.h +++ b/src/ucm/ucm_local.h @@ -47,6 +47,7 @@ #define SEQUENCE_ELEMENT_TYPE_CSET 2 #define SEQUENCE_ELEMENT_TYPE_SLEEP 3 #define SEQUENCE_ELEMENT_TYPE_EXEC 4 +#define SEQUENCE_ELEMENT_TYPE_BCSETF 5
struct ucm_value { struct list_head list; @@ -63,6 +64,7 @@ struct sequence_element { char *cset; char *exec; } data;
- int isbin; /* Indicate cset is binary array or ascii array */
};
/*
2.1.0
On Tue, 2015-01-13 at 20:50 +0100, Takashi Iwai wrote:
At Tue, 13 Jan 2015 18:26:18 +0000, Liam Girdwood wrote:
On Tue, 2015-01-13 at 17:52 +0100, Takashi Iwai wrote:
At Tue, 13 Jan 2015 11:00:39 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
with cset command, UCM set kcontrol parameters directly: cset "name='<KCONTROL_NAME>' 1,2<,3,...>" This patch enables UCM to set kcontrol with parameters from configure file: bcsetf "name='<KCONTROL_NAME>' <path/to/file>" where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).
Why binary? It's not portable. You can't carry it to a different architecture.
The intention here is that the binary data is not meant for the host but for audio DSPs so it's just passed by UCM/ALSA as raw data.
In that case, we should limit to certain element data types. Otherwise people would abuse it for passing data even to integer or enum ctls.
And of course it'd be better to clarify the reason in the patch description :)
BTW, I'm still not so convinced by bcsetf... Can't it be more verbose or readable?
Yeah, we should probably make it more readable :)
Lu Han will probably have some naming suggestions shortly....
Liam
At Tue, 13 Jan 2015 11:00:38 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Signed-off-by: Lu, Han han.lu@intel.com
Looks good to me. Liam, any objection for this extension?
thanks,
Takashi
diff --git a/src/control/ctlparse.c b/src/control/ctlparse.c index 978977d..acaf734 100644 --- a/src/control/ctlparse.c +++ b/src/control/ctlparse.c @@ -59,10 +59,10 @@ static long get_integer(const char **ptr, long min, long max) goto out;
s = p;
- val = strtol(s, &p, 10);
- val = strtol(s, &p, 0); if (*p == '.') { p++;
strtol(p, &p, 10);
strtol(p, &p, 0);@@ -87,10 +87,10 @@ static long long get_integer64(const char **ptr, long long min, long long max) goto out;
s = p;
- val = strtol(s, &p, 10);
- val = strtol(s, &p, 0); if (*p == '.') { p++;
strtol(p, &p, 10);
strtol(p, &p, 0);-- 2.1.0
On Tue, 2015-01-13 at 17:53 +0100, Takashi Iwai wrote:
At Tue, 13 Jan 2015 11:00:38 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Signed-off-by: Lu, Han han.lu@intel.com
Looks good to me. Liam, any objection for this extension?
Acked-by: Liam Girdwood liam.r.girdwood@linux.intel.com
thanks,
Takashi
diff --git a/src/control/ctlparse.c b/src/control/ctlparse.c index 978977d..acaf734 100644 --- a/src/control/ctlparse.c +++ b/src/control/ctlparse.c @@ -59,10 +59,10 @@ static long get_integer(const char **ptr, long min, long max) goto out;
s = p;
- val = strtol(s, &p, 10);
- val = strtol(s, &p, 0); if (*p == '.') { p++;
strtol(p, &p, 10);
strtol(p, &p, 0);@@ -87,10 +87,10 @@ static long long get_integer64(const char **ptr, long long min, long long max) goto out;
s = p;
- val = strtol(s, &p, 10);
- val = strtol(s, &p, 0); if (*p == '.') { p++;
strtol(p, &p, 10);
strtol(p, &p, 0);-- 2.1.0
At Tue, 13 Jan 2015 17:53:14 +0100, Takashi Iwai wrote:
At Tue, 13 Jan 2015 11:00:38 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Signed-off-by: Lu, Han han.lu@intel.com
Looks good to me. Liam, any objection for this extension?
Erm, sorry, I correct my statement: this is buggy. Look at the code:
- val = strtol(s, &p, 10);
- val = strtol(s, &p, 0); if (*p == '.') { p++;
strtol(p, &p, 10);
strtol(p, &p, 0);
The second strtol() is for skipping the decimals. So this has to be 10-based. That is, use zero-base only for the first strtol().
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, January 14, 2015 4:02 AM To: Takashi Iwai Cc: Lu, Han; Liam Girdwood; alsa-devel@alsa-project.org Subject: Re: [PATCH - UCM 1/2] control: enable octal and hexadecimal parse
At Tue, 13 Jan 2015 17:53:14 +0100, Takashi Iwai wrote:
At Tue, 13 Jan 2015 11:00:38 +0800, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Signed-off-by: Lu, Han han.lu@intel.com
Looks good to me. Liam, any objection for this extension?
Erm, sorry, I correct my statement: this is buggy. Look at the code:
- val = strtol(s, &p, 10);
- val = strtol(s, &p, 0); if (*p == '.') { p++;
strtol(p, &p, 10);
strtol(p, &p, 0);The second strtol() is for skipping the decimals. So this has to be 10-based. That is, use zero-base only for the first strtol().
Yes. I thought of skipping hexadecimal, but string begin with ".0x" looks not reasonable, and string begin with ".0" will not be skipped as expected with this patch. I have removed the second change and resend the patch. Please review, Thanks.
BR, Han Lu
Takashi
participants (4)
-
han.lu＠intel.com -
Liam Girdwood -
Lu, Han
-
Takashi Iwai