* [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function
@ 2019-12-18 12:25 Pengfei Xu
2019-12-18 12:25 ` [LTP] [PATCH 2/2] umip_basic_test.c: improve kconfig check to avoid umip wrong abort case Pengfei Xu
2019-12-19 4:02 ` [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Yang Xu
0 siblings, 2 replies; 8+ messages in thread
From: Pengfei Xu @ 2019-12-18 12:25 UTC (permalink / raw)
To: ltp
Add "or" select kconfig function:
For example, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
to "CONFIG_X86_UMIP=y": as before v5.4 mainline kernel used
kconfig "CONFIG_X86_INTEL_UMIP=y", after v5.5 mainline kernel would use
"CONFIG_X86_UMIP=y".
We could fill "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" in needs_kconfigs
to check umip kconfig item, which actually is the same item.
Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
---
lib/tst_kconfig.c | 43 +++++++++++++++++++++++++++----------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
index 4b51413e5..91c0a821b 100644
--- a/lib/tst_kconfig.c
+++ b/lib/tst_kconfig.c
@@ -167,7 +167,12 @@ void tst_kconfig_read(const char *const *kconfigs,
struct match matches[cnt];
FILE *fp;
unsigned int i, j;
- char buf[1024];
+ char buf[1024], kconfig_multi[100];
+ char *kconfig_token = NULL, *p_left = NULL;
+
+ fp = open_kconfig();
+ if (!fp)
+ tst_brk(TBROK, "Cannot parse kernel .config");
for (i = 0; i < cnt; i++) {
const char *val = strchr(kconfigs[i], '=');
@@ -178,32 +183,38 @@ void tst_kconfig_read(const char *const *kconfigs,
matches[i].match = 0;
matches[i].len = strlen(kconfigs[i]);
- if (val) {
+ if (val)
matches[i].val = val + 1;
- matches[i].len -= strlen(val);
- }
results[i].match = 0;
results[i].value = NULL;
- }
- fp = open_kconfig();
- if (!fp)
- tst_brk(TBROK, "Cannot parse kernel .config");
+ while (fgets(buf, sizeof(buf), fp)) {
- while (fgets(buf, sizeof(buf), fp)) {
- for (i = 0; i < cnt; i++) {
- if (match(&matches[i], kconfigs[i], &results[i], buf)) {
- for (j = 0; j < cnt; j++) {
- if (matches[j].match)
- break;
- }
+ memset(kconfig_multi, 0, sizeof(kconfig_multi));
+ /* strtok_r will split kconfigs[i] to multi string, so copy it */
+ memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
+
+ kconfig_token = strtok_r(kconfig_multi, "|=", &p_left);
+ while (kconfig_token != NULL) {
+ if (strncmp("CONFIG_", kconfig_token, 7))
+ tst_brk(TBROK, "Invalid config string '%s'", kconfig_token);
+ matches[i].len = strlen(kconfig_token);
+ if (match(&matches[i], kconfig_token, &results[i], buf)) {
+ for (j = 0; j < cnt; j++) {
+ if (matches[j].match)
+ break;
+ }
if (j == cnt)
goto exit;
+ }
+ kconfig_token = strtok_r(NULL, "|=", &p_left);
+ /* avoid to use after "=" string */
+ if (strlen(p_left) == 0)
+ break;
}
}
-
}
exit:
--
2.14.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 2/2] umip_basic_test.c: improve kconfig check to avoid umip wrong abort case
2019-12-18 12:25 [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Pengfei Xu
@ 2019-12-18 12:25 ` Pengfei Xu
2019-12-19 4:02 ` [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Yang Xu
1 sibling, 0 replies; 8+ messages in thread
From: Pengfei Xu @ 2019-12-18 12:25 UTC (permalink / raw)
To: ltp
From v5.5 main line, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
to "CONFIG_X86_UMIP=y".
We could use "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" to check kconfig
CONFIG_X86_INTEL_UMIP=y(old kernel) or CONFIG_X86_UMIP=y(new kernel) for umip.
Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
---
testcases/kernel/security/umip/umip_basic_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/testcases/kernel/security/umip/umip_basic_test.c b/testcases/kernel/security/umip/umip_basic_test.c
index 1baa26c52..24eca8890 100644
--- a/testcases/kernel/security/umip/umip_basic_test.c
+++ b/testcases/kernel/security/umip/umip_basic_test.c
@@ -171,7 +171,7 @@ static struct tst_test test = {
.forks_child = 1,
.test = verify_umip_instruction,
.needs_kconfigs = (const char *[]){
- "CONFIG_X86_INTEL_UMIP=y",
+ "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",
NULL
},
.needs_root = 1,
--
2.14.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function
2019-12-18 12:25 [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Pengfei Xu
2019-12-18 12:25 ` [LTP] [PATCH 2/2] umip_basic_test.c: improve kconfig check to avoid umip wrong abort case Pengfei Xu
@ 2019-12-19 4:02 ` Yang Xu
2019-12-19 9:05 ` Yang Xu
2019-12-19 10:03 ` Pengfei Xu
1 sibling, 2 replies; 8+ messages in thread
From: Yang Xu @ 2019-12-19 4:02 UTC (permalink / raw)
To: ltp
Hi Pengfei
on 2019/12/18 20:25, Pengfei Xu wrote:
> Add "or" select kconfig function:
> For example, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
> to "CONFIG_X86_UMIP=y": as before v5.4 mainline kernel used
> kconfig "CONFIG_X86_INTEL_UMIP=y", after v5.5 mainline kernel would use
> "CONFIG_X86_UMIP=y".
> We could fill "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" in needs_kconfigs
> to check umip kconfig item, which actually is the same item.
>
> Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
> ---
> lib/tst_kconfig.c | 43 +++++++++++++++++++++++++++----------------
> 1 file changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 4b51413e5..91c0a821b 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -167,7 +167,12 @@ void tst_kconfig_read(const char *const *kconfigs,
> struct match matches[cnt];
> FILE *fp;
> unsigned int i, j;
> - char buf[1024];
> + char buf[1024], kconfig_multi[100];
> + char *kconfig_token = NULL, *p_left = NULL;
> +
> + fp = open_kconfig();
> + if (!fp)
> + tst_brk(TBROK, "Cannot parse kernel .config");
>
> for (i = 0; i < cnt; i++) {
> const char *val = strchr(kconfigs[i], '=');
> @@ -178,32 +183,38 @@ void tst_kconfig_read(const char *const *kconfigs,
> matches[i].match = 0;
> matches[i].len = strlen(kconfigs[i]);
>
> - if (val) {
> + if (val)
> matches[i].val = val + 1;
> - matches[i].len -= strlen(val);
> - }
>
> results[i].match = 0;
> results[i].value = NULL;
> - }
>
> - fp = open_kconfig();
> - if (!fp)
> - tst_brk(TBROK, "Cannot parse kernel .config");
> + while (fgets(buf, sizeof(buf), fp)) {
>
> - while (fgets(buf, sizeof(buf), fp)) {
> - for (i = 0; i < cnt; i++) {
> - if (match(&matches[i], kconfigs[i], &results[i], buf)) {
> - for (j = 0; j < cnt; j++) {
> - if (matches[j].match)
> - break;
> - }
> + memset(kconfig_multi, 0, sizeof(kconfig_multi));
> + /* strtok_r will split kconfigs[i] to multi string, so copy it */
> + memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
> +
> + kconfig_token = strtok_r(kconfig_multi, "|=", &p_left)
Here has a problem, if we use CONFIG_X86_INTEL_UMIP, it will report
"miss this config" because it uses "=" or "|" to delim string.
And I think you should use lib/newlib_tests/test_kconfig.c to test your
introduced feature.
Also, it has another two problems even we use "|" or "=" in kconfigs
1.If use "CONFIG_X86_INTEL_UMIP=y|CONFIG_X86_UMIP=y" ,it will report y
is invalid because we use "="or "|" to delim string.
2. If use "CONFIG_X86_INTEL_UMIP|X86_INTEL_UMIP=y", it will doesn't
check second config whether invalid if the first is ok.
Kind Regards
Yang Xu
> + while (kconfig_token != NULL) {
> + if (strncmp("CONFIG_", kconfig_token, 7))
> + tst_brk(TBROK, "Invalid config string '%s'", kconfig_token);
> + matches[i].len = strlen(kconfig_token);
> + if (match(&matches[i], kconfig_token, &results[i], buf)) {
> + for (j = 0; j < cnt; j++) {
> + if (matches[j].match)
> + break;
> + }
>
> if (j == cnt)
> goto exit;
> + }
> + kconfig_token = strtok_r(NULL, "|=", &p_left);
> + /* avoid to use after "=" string */
> + if (strlen(p_left) == 0)
> + break;
> }
> }
> -
> }
>
> exit:
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function
2019-12-19 4:02 ` [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Yang Xu
@ 2019-12-19 9:05 ` Yang Xu
2019-12-19 10:06 ` Pengfei Xu
2019-12-19 10:03 ` Pengfei Xu
1 sibling, 1 reply; 8+ messages in thread
From: Yang Xu @ 2019-12-19 9:05 UTC (permalink / raw)
To: ltp
Hi Pengfei
on 2019/12/19 12:02, Yang Xu wrote:
> Hi Pengfei
>
> on 2019/12/18 20:25, Pengfei Xu wrote:
>> Add "or" select kconfig function:
>> ?? For example, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
>> to "CONFIG_X86_UMIP=y": as before v5.4 mainline kernel used
>> kconfig "CONFIG_X86_INTEL_UMIP=y", after v5.5 mainline kernel would use
>> "CONFIG_X86_UMIP=y".
>> ?? We could fill "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" in
>> needs_kconfigs
>> to check umip kconfig item, which actually is the same item.
>>
Or, we can only modify match funtion to make it possibile. What do you
think about it?
The way as bleow:
1. detect whether has "|"
2. strncmp system kconfig with our first
kconfig(CONFIG_X86_INTEL_UMIP), if not ,compare with the second
kconfig(CONFIG_X86_UMIP)
diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
index 4b51413e5..cb9ee46bf 100644
--- a/lib/tst_kconfig.c
+++ b/lib/tst_kconfig.c
@@ -117,20 +117,41 @@ static int is_set(const char *str, const char *val)
static inline int match(struct match *match, const char *conf,
struct tst_kconfig_res *result, const char *line)
{
+ int len, len1 = 0;
if (match->match)
return 0;
-
+ len1 = match->len;
const char *cfg = strstr(line, "CONFIG_");
if (!cfg)
return 0;
- if (strncmp(cfg, conf, match->len))
- return 0;
-
- const char *val = &cfg[match->len];
-
- switch (cfg[match->len]) {
+ const char * val1 = strchr(conf, '|');
+ if (!val1) {
+ if (strncmp(cfg, conf, match->len))
+ return 0;
+ } else {
+ const char *val3 = strchr(val1, '=');
+ const char *val2 = strstr(val1, "CONFIG_");
+ if (!val2) {
+ tst_brk(TBROK, "Invalid config string '%s'", val1);
+ return 0;
+ }
+ if(!val3)
+ len = strlen(val2);
+ else
+ len = strlen(val2)-strlen(val3);
+
+ if (strncmp(cfg, conf, match->len - (len+1))) {
+ if (strncmp(cfg, val2, len)) {
+ return 0;
+ }
+ len1 = len;
+ } else
+ len1 = match->len - len -1;
+ }
+ const char *val = &cfg[len1];
+ switch (cfg[len1]) {
Kind Regards
Yang Xu
>> Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
>> ---
>> ? lib/tst_kconfig.c | 43 +++++++++++++++++++++++++++----------------
>> ? 1 file changed, 27 insertions(+), 16 deletions(-)
>>
>> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
>> index 4b51413e5..91c0a821b 100644
>> --- a/lib/tst_kconfig.c
>> +++ b/lib/tst_kconfig.c
>> @@ -167,7 +167,12 @@ void tst_kconfig_read(const char *const *kconfigs,
>> ????? struct match matches[cnt];
>> ????? FILE *fp;
>> ????? unsigned int i, j;
>> -??? char buf[1024];
>> +??? char buf[1024], kconfig_multi[100];
>> +??? char *kconfig_token = NULL, *p_left = NULL;
>> +
>> +??? fp = open_kconfig();
>> +??? if (!fp)
>> +??????? tst_brk(TBROK, "Cannot parse kernel .config");
>> ????? for (i = 0; i < cnt; i++) {
>> ????????? const char *val = strchr(kconfigs[i], '=');
>> @@ -178,32 +183,38 @@ void tst_kconfig_read(const char *const *kconfigs,
>> ????????? matches[i].match = 0;
>> ????????? matches[i].len = strlen(kconfigs[i]);
>> -??????? if (val) {
>> +??????? if (val)
>> ????????????? matches[i].val = val + 1;
>> -??????????? matches[i].len -= strlen(val);
>> -??????? }
>> ????????? results[i].match = 0;
>> ????????? results[i].value = NULL;
>> -??? }
>> -??? fp = open_kconfig();
>> -??? if (!fp)
>> -??????? tst_brk(TBROK, "Cannot parse kernel .config");
>> +??????? while (fgets(buf, sizeof(buf), fp)) {
>> -??? while (fgets(buf, sizeof(buf), fp)) {
>> -??????? for (i = 0; i < cnt; i++) {
>> -??????????? if (match(&matches[i], kconfigs[i], &results[i], buf)) {
>> -??????????????? for (j = 0; j < cnt; j++) {
>> -??????????????????? if (matches[j].match)
>> -??????????????????????? break;
>> -??????????????? }
>> +??????????? memset(kconfig_multi, 0, sizeof(kconfig_multi));
>> +??????????? /* strtok_r will split kconfigs[i] to multi string, so
>> copy it */
>> +??????????? memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
>> +
>> +??????????? kconfig_token = strtok_r(kconfig_multi, "|=", &p_left)
> Here has a problem, if we use CONFIG_X86_INTEL_UMIP, it will report
> "miss this config" because it uses "=" or "|" to delim string.
> And I think you should use lib/newlib_tests/test_kconfig.c to test your
> introduced feature.
>
> Also, it has another two problems even we use "|" or "=" in kconfigs
>
> 1.If? use "CONFIG_X86_INTEL_UMIP=y|CONFIG_X86_UMIP=y" ,it will report y
> is invalid because we use "="or "|" to delim string.
> 2. If? use "CONFIG_X86_INTEL_UMIP|X86_INTEL_UMIP=y", it will doesn't
> check second config whether invalid if the first is ok.
>
> Kind Regards
> Yang Xu
>> +??????????? while (kconfig_token != NULL) {
>> +??????????????? if (strncmp("CONFIG_", kconfig_token, 7))
>> +??????????????????? tst_brk(TBROK, "Invalid config string '%s'",
>> kconfig_token);
>> +??????????????? matches[i].len = strlen(kconfig_token);
>> +??????????????? if (match(&matches[i], kconfig_token, &results[i],
>> buf)) {
>> +??????????????????? for (j = 0; j < cnt; j++) {
>> +??????????????????????? if (matches[j].match)
>> +??????????????????????????? break;
>> +??????????????????? }
>> ????????????????? if (j == cnt)
>> ????????????????????? goto exit;
>> +??????????????? }
>> +??????????????? kconfig_token = strtok_r(NULL, "|=", &p_left);
>> +??????????????? /* avoid to use after "=" string */
>> +??????????????? if (strlen(p_left) == 0)
>> +??????????????????? break;
>> ????????????? }
>> ????????? }
>> -
>> ????? }
>> ? exit:
>>
>
>
>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function
2019-12-19 4:02 ` [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Yang Xu
2019-12-19 9:05 ` Yang Xu
@ 2019-12-19 10:03 ` Pengfei Xu
1 sibling, 0 replies; 8+ messages in thread
From: Pengfei Xu @ 2019-12-19 10:03 UTC (permalink / raw)
To: ltp
Hi Xu,
Thanks for your comments!
I made one wrong loop, which caused this issue.
I will make another patch for any KCONFIG_AA or KCONFIG_AB is matached with
y feature. : )
Thanks!
BR.
On 2019-12-19 at 12:02:34 +0800, Yang Xu wrote:
> Hi Pengfei
>
> on 2019/12/18 20:25, Pengfei Xu wrote:
> > - fp = open_kconfig();
> > - if (!fp)
> > - tst_brk(TBROK, "Cannot parse kernel .config");
> > + while (fgets(buf, sizeof(buf), fp)) {
> > - while (fgets(buf, sizeof(buf), fp)) {
> > - for (i = 0; i < cnt; i++) {
> > - if (match(&matches[i], kconfigs[i], &results[i], buf)) {
> > - for (j = 0; j < cnt; j++) {
> > - if (matches[j].match)
> > - break;
> > - }
> > + memset(kconfig_multi, 0, sizeof(kconfig_multi));
> > + /* strtok_r will split kconfigs[i] to multi string, so copy it */
> > + memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
> > +
> > + kconfig_token = strtok_r(kconfig_multi, "|=", &p_left)
> Here has a problem, if we use CONFIG_X86_INTEL_UMIP, it will report "miss
> this config" because it uses "=" or "|" to delim string.
> And I think you should use lib/newlib_tests/test_kconfig.c to test your
> introduced feature.
>
> Also, it has another two problems even we use "|" or "=" in kconfigs
>
> 1.If use "CONFIG_X86_INTEL_UMIP=y|CONFIG_X86_UMIP=y" ,it will report y is
> invalid because we use "="or "|" to delim string.
> 2. If use "CONFIG_X86_INTEL_UMIP|X86_INTEL_UMIP=y", it will doesn't check
> second config whether invalid if the first is ok.
>
> Kind Regards
> Yang Xu
> > + while (kconfig_token != NULL) {
> > + if (strncmp("CONFIG_", kconfig_token, 7))
> > + tst_brk(TBROK, "Invalid config string '%s'", kconfig_token);
> > + matches[i].len = strlen(kconfig_token);
> > + if (match(&matches[i], kconfig_token, &results[i], buf)) {
> > + for (j = 0; j < cnt; j++) {
> > + if (matches[j].match)
> > + break;
> > + }
> > if (j == cnt)
> > goto exit;
> > + }
> > + kconfig_token = strtok_r(NULL, "|=", &p_left);
> > + /* avoid to use after "=" string */
> > + if (strlen(p_left) == 0)
> > + break;
> > }
> > }
> > -
> > }
> > exit:
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function
2019-12-19 9:05 ` Yang Xu
@ 2019-12-19 10:06 ` Pengfei Xu
2019-12-19 11:03 ` Yang Xu
0 siblings, 1 reply; 8+ messages in thread
From: Pengfei Xu @ 2019-12-19 10:06 UTC (permalink / raw)
To: ltp
Hi Xu,
Thanks for your advice!
My concern is that:
If some one need any CONFIG_A, CONFIG_B, CONFIG_C ... set to y.
We need rewrite again.
Also, thanks for advice, I will used newlib_tests to double check. :)
Thanks!
BR.
On 2019-12-19 at 17:05:00 +0800, Yang Xu wrote:
> Hi Pengfei
>
> on 2019/12/19 12:02, Yang Xu wrote:
> > Hi Pengfei
> >
> > on 2019/12/18 20:25, Pengfei Xu wrote:
> > > Add "or" select kconfig function:
> > > ?? For example, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
> > > to "CONFIG_X86_UMIP=y": as before v5.4 mainline kernel used
> > > kconfig "CONFIG_X86_INTEL_UMIP=y", after v5.5 mainline kernel would use
> > > "CONFIG_X86_UMIP=y".
> > > ?? We could fill "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" in
> > > needs_kconfigs
> > > to check umip kconfig item, which actually is the same item.
> > >
> Or, we can only modify match funtion to make it possibile. What do you think
> about it?
> The way as bleow:
> 1. detect whether has "|"
> 2. strncmp system kconfig with our first kconfig(CONFIG_X86_INTEL_UMIP), if
> not ,compare with the second kconfig(CONFIG_X86_UMIP)
>
> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 4b51413e5..cb9ee46bf 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -117,20 +117,41 @@ static int is_set(const char *str, const char *val)
> static inline int match(struct match *match, const char *conf,
> struct tst_kconfig_res *result, const char *line)
> {
> + int len, len1 = 0;
> if (match->match)
> return 0;
> -
> + len1 = match->len;
> const char *cfg = strstr(line, "CONFIG_");
>
> if (!cfg)
> return 0;
>
> - if (strncmp(cfg, conf, match->len))
> - return 0;
> -
> - const char *val = &cfg[match->len];
> -
> - switch (cfg[match->len]) {
> + const char * val1 = strchr(conf, '|');
> + if (!val1) {
> + if (strncmp(cfg, conf, match->len))
> + return 0;
> + } else {
> + const char *val3 = strchr(val1, '=');
> + const char *val2 = strstr(val1, "CONFIG_");
> + if (!val2) {
> + tst_brk(TBROK, "Invalid config string '%s'", val1);
> + return 0;
> + }
> + if(!val3)
> + len = strlen(val2);
> + else
> + len = strlen(val2)-strlen(val3);
> +
> + if (strncmp(cfg, conf, match->len - (len+1))) {
> + if (strncmp(cfg, val2, len)) {
> + return 0;
> + }
> + len1 = len;
> + } else
> + len1 = match->len - len -1;
> + }
> + const char *val = &cfg[len1];
> + switch (cfg[len1]) {
>
>
> Kind Regards
> Yang Xu
> > > Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
> > > ---
> > > ? lib/tst_kconfig.c | 43 +++++++++++++++++++++++++++----------------
> > > ? 1 file changed, 27 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> > > index 4b51413e5..91c0a821b 100644
> > > --- a/lib/tst_kconfig.c
> > > +++ b/lib/tst_kconfig.c
> > > @@ -167,7 +167,12 @@ void tst_kconfig_read(const char *const *kconfigs,
> > > ????? struct match matches[cnt];
> > > ????? FILE *fp;
> > > ????? unsigned int i, j;
> > > -??? char buf[1024];
> > > +??? char buf[1024], kconfig_multi[100];
> > > +??? char *kconfig_token = NULL, *p_left = NULL;
> > > +
> > > +??? fp = open_kconfig();
> > > +??? if (!fp)
> > > +??????? tst_brk(TBROK, "Cannot parse kernel .config");
> > > ????? for (i = 0; i < cnt; i++) {
> > > ????????? const char *val = strchr(kconfigs[i], '=');
> > > @@ -178,32 +183,38 @@ void tst_kconfig_read(const char *const *kconfigs,
> > > ????????? matches[i].match = 0;
> > > ????????? matches[i].len = strlen(kconfigs[i]);
> > > -??????? if (val) {
> > > +??????? if (val)
> > > ????????????? matches[i].val = val + 1;
> > > -??????????? matches[i].len -= strlen(val);
> > > -??????? }
> > > ????????? results[i].match = 0;
> > > ????????? results[i].value = NULL;
> > > -??? }
> > > -??? fp = open_kconfig();
> > > -??? if (!fp)
> > > -??????? tst_brk(TBROK, "Cannot parse kernel .config");
> > > +??????? while (fgets(buf, sizeof(buf), fp)) {
> > > -??? while (fgets(buf, sizeof(buf), fp)) {
> > > -??????? for (i = 0; i < cnt; i++) {
> > > -??????????? if (match(&matches[i], kconfigs[i], &results[i], buf)) {
> > > -??????????????? for (j = 0; j < cnt; j++) {
> > > -??????????????????? if (matches[j].match)
> > > -??????????????????????? break;
> > > -??????????????? }
> > > +??????????? memset(kconfig_multi, 0, sizeof(kconfig_multi));
> > > +??????????? /* strtok_r will split kconfigs[i] to multi string, so
> > > copy it */
> > > +??????????? memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
> > > +
> > > +??????????? kconfig_token = strtok_r(kconfig_multi, "|=", &p_left)
> > Here has a problem, if we use CONFIG_X86_INTEL_UMIP, it will report
> > "miss this config" because it uses "=" or "|" to delim string.
> > And I think you should use lib/newlib_tests/test_kconfig.c to test your
> > introduced feature.
> >
> > Also, it has another two problems even we use "|" or "=" in kconfigs
> >
> > 1.If? use "CONFIG_X86_INTEL_UMIP=y|CONFIG_X86_UMIP=y" ,it will report y
> > is invalid because we use "="or "|" to delim string.
> > 2. If? use "CONFIG_X86_INTEL_UMIP|X86_INTEL_UMIP=y", it will doesn't
> > check second config whether invalid if the first is ok.
> >
> > Kind Regards
> > Yang Xu
> > > +??????????? while (kconfig_token != NULL) {
> > > +??????????????? if (strncmp("CONFIG_", kconfig_token, 7))
> > > +??????????????????? tst_brk(TBROK, "Invalid config string '%s'",
> > > kconfig_token);
> > > +??????????????? matches[i].len = strlen(kconfig_token);
> > > +??????????????? if (match(&matches[i], kconfig_token, &results[i],
> > > buf)) {
> > > +??????????????????? for (j = 0; j < cnt; j++) {
> > > +??????????????????????? if (matches[j].match)
> > > +??????????????????????????? break;
> > > +??????????????????? }
> > > ????????????????? if (j == cnt)
> > > ????????????????????? goto exit;
> > > +??????????????? }
> > > +??????????????? kconfig_token = strtok_r(NULL, "|=", &p_left);
> > > +??????????????? /* avoid to use after "=" string */
> > > +??????????????? if (strlen(p_left) == 0)
> > > +??????????????????? break;
> > > ????????????? }
> > > ????????? }
> > > -
> > > ????? }
> > > ? exit:
> > >
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function
2019-12-19 10:06 ` Pengfei Xu
@ 2019-12-19 11:03 ` Yang Xu
2019-12-19 13:18 ` Pengfei Xu
0 siblings, 1 reply; 8+ messages in thread
From: Yang Xu @ 2019-12-19 11:03 UTC (permalink / raw)
To: ltp
Hi Pengfei
> Hi Xu,
> Thanks for your advice!
> My concern is that:
> If some one need any CONFIG_A, CONFIG_B, CONFIG_C ... set to y.
> We need rewrite again.
> Also, thanks for advice, I will used newlib_tests to double check.:)
>
I see. Also, you should add example to lib/newlib_tests/test_kconfig.c
so we can know the kconfig feature is ok when maintainer merges your
patch. Usually, if we modify or add lib feature, we will document it in
doc/test-writing-guidelines.txt. So if you do it , it will be better.
as below:
diff --git a/lib/newlib_tests/test_kconfig.c
b/lib/newlib_tests/test_kconfig.c
index d9c662fc5..9515b60e2 100644
--- a/lib/newlib_tests/test_kconfig.c
+++ b/lib/newlib_tests/test_kconfig.c
@@ -14,6 +14,7 @@ static const char *kconfigs[] = {
"CONFIG_MMU",
"CONFIG_EXT4_FS=m",
"CONFIG_PGTABLE_LEVELS=4",
+ "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",
NULL
};
Kind Regards
Yang Xu
> Thanks!
> BR.
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function
2019-12-19 11:03 ` Yang Xu
@ 2019-12-19 13:18 ` Pengfei Xu
0 siblings, 0 replies; 8+ messages in thread
From: Pengfei Xu @ 2019-12-19 13:18 UTC (permalink / raw)
To: ltp
Hi Xu,
On 2019-12-19 at 19:03:16 +0800, Yang Xu wrote:
> Hi Pengfei
> > Hi Xu,
> > Thanks for your advice!
> > My concern is that:
> > If some one need any CONFIG_A, CONFIG_B, CONFIG_C ... set to y.
> > We need rewrite again.
> > Also, thanks for advice, I will used newlib_tests to double check.:)
> >
> I see. Also, you should add example to lib/newlib_tests/test_kconfig.c so we
> can know the kconfig feature is ok when maintainer merges your patch.
> Usually, if we modify or add lib feature, we will document it in
> doc/test-writing-guidelines.txt. So if you do it , it will be better.
> as below:
> diff --git a/lib/newlib_tests/test_kconfig.c
> b/lib/newlib_tests/test_kconfig.c
> index d9c662fc5..9515b60e2 100644
> --- a/lib/newlib_tests/test_kconfig.c
> +++ b/lib/newlib_tests/test_kconfig.c
> @@ -14,6 +14,7 @@ static const char *kconfigs[] = {
> "CONFIG_MMU",
> "CONFIG_EXT4_FS=m",
> "CONFIG_PGTABLE_LEVELS=4",
> + "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",
> NULL
> };
Thanks for your advice! I will add them.
Thanks!
BR.
>
> Kind Regards
> Yang Xu
> > Thanks!
> > BR.
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-12-19 13:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 12:25 [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Pengfei Xu
2019-12-18 12:25 ` [LTP] [PATCH 2/2] umip_basic_test.c: improve kconfig check to avoid umip wrong abort case Pengfei Xu
2019-12-19 4:02 ` [LTP] [PATCH 1/2] lib/tst_kconfig.c: add or select kconfig function Yang Xu
2019-12-19 9:05 ` Yang Xu
2019-12-19 10:06 ` Pengfei Xu
2019-12-19 11:03 ` Yang Xu
2019-12-19 13:18 ` Pengfei Xu
2019-12-19 10:03 ` Pengfei Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.