<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 14, 2019 at 7:25 PM Cyril Hrubis <<a href="mailto:chrubis@suse.cz">chrubis@suse.cz</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The newly introduced test tags are generic name-value pairs that can<br>
hold test metadata, the intended use for now is to store kernel commit<br>
hashes for kernel reproducers as well as CVE ids. The mechanism is<br>
however choosen to be very generic so that it's easy to add basically<br>
any information later on.<br>
<br>
As it is the main purpose is to print hints for a test failures. If a<br>
test that has been written as a kernel reproducer fails it prints nice<br>
URL pointing to a kernel commit that may be missing.<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">Nice to see this feature patchset achieved, it looks quite clear&good to me. Just have some tiny comments below.</div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Example output:<br>
<br>
--------------------------------------------------------------------------<br>
tst_test.c:1145: INFO: Timeout per run is 0h 05m 00s<br>
add_key02.c:98: FAIL: unexpected error with key type 'asymmetric': EINVAL<br>
add_key02.c:98: FAIL: unexpected error with key type 'cifs.idmap': EINVAL<br>
add_key02.c:98: FAIL: unexpected error with key type 'cifs.spnego': EINVAL<br>
add_key02.c:98: FAIL: unexpected error with key type 'pkcs7_test': EINVAL<br>
add_key02.c:98: FAIL: unexpected error with key type 'rxrpc': EINVAL<br>
add_key02.c:98: FAIL: unexpected error with key type 'rxrpc_s': EINVAL<br>
add_key02.c:98: FAIL: unexpected error with key type 'user': EINVAL<br>
add_key02.c:98: FAIL: unexpected error with key type 'logon': EINVAL<br>
<br>
HINT: This is a regression test for linux kernel, see commit:<br>
<br>
<a href="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5649645d725c" rel="noreferrer" target="_blank">https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5649645d725c</a><br>
<br>
HINT: This test also tests for CVE-2017-15274, see:<br>
<br>
<a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15274" rel="noreferrer" target="_blank">https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15274</a><br>
<br>
Summary:<br>
passed 0<br>
failed 8<br>
skipped 0<br>
warnings 0<br>
--------------------------------------------------------------------------<br>
<br>
Signed-off-by: Cyril Hrubis <<a href="mailto:chrubis@suse.cz" target="_blank">chrubis@suse.cz</a>><br>
---<br>
include/tst_test.h | 10 ++++++<br>
lib/tst_test.c | 77 +++++++++++++++++++++++++++++++++++++++++-----<br>
2 files changed, 80 insertions(+), 7 deletions(-)<br>
<br>
diff --git a/include/tst_test.h b/include/tst_test.h<br>
index 84acf2c59..4a51b6d16 100644<br>
--- a/include/tst_test.h<br>
+++ b/include/tst_test.h<br>
@@ -108,6 +108,11 @@ int tst_parse_int(const char *str, int *val, int min, int max);<br>
int tst_parse_long(const char *str, long *val, long min, long max);<br>
int tst_parse_float(const char *str, float *val, float min, float max);<br>
<br>
+struct tst_tag {<br>
+ const char *name;<br>
+ const char *value;<br>
+};<br>
+<br>
extern unsigned int tst_variant;<br>
<br>
struct tst_test {<br>
@@ -212,6 +217,11 @@ struct tst_test {<br>
* NULL-terminated array of capability settings<br>
*/<br>
struct tst_cap *caps;<br>
+<br>
+ /*<br>
+ * {NULL, NULL} terminated array of tags.<br>
+ */<br>
+ const struct tst_tag *tags;<br>
};<br>
<br>
/*<br>
diff --git a/lib/tst_test.c b/lib/tst_test.c<br>
index 6239acf89..2129f38cb 100644<br>
--- a/lib/tst_test.c<br>
+++ b/lib/tst_test.c<br>
@@ -31,6 +31,9 @@<br>
#include "old_device.h"<br>
#include "old_tmpdir.h"<br>
<br>
+#define LINUX_GIT_URL "<a href="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=" rel="noreferrer" target="_blank">https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=</a>"<br>
+#define CVE_DB_URL "<a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-" rel="noreferrer" target="_blank">https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-</a>"<br>
+<br>
struct tst_test *tst_test;<br>
<br>
static const char *tid;<br>
@@ -414,6 +417,9 @@ static void print_help(void)<br>
{<br>
unsigned int i;<br>
<br>
+ fprintf(stderr, "Options\n");<br>
+ fprintf(stderr, "-------\n\n");<br>
+<br>
for (i = 0; i < ARRAY_SIZE(options); i++)<br>
fprintf(stderr, "%s\n", options[i].help);<br>
<br>
@@ -424,6 +430,27 @@ static void print_help(void)<br>
fprintf(stderr, "%s\n", tst_test->options[i].help);<br>
}<br>
<br>
+static void print_test_info(void)<br></blockquote><div><br></div><div class="gmail_default" style="font-size:small">print_test_info sounds like general information for the test, maybe print_tags() is a better/precise name?</div><div class="gmail_default" style="font-size:small"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+{<br>
+ unsigned int i;<br>
+ const struct tst_tag *tags = tst_test->tags;<br>
+<br>
+ printf("\nTags\n");<br>
+ printf("----\n\n");<br>
+<br>
+ if (tags) {<br>
+ for (i = 0; tags[i].name; i++) {<br>
+ if (!strcmp(tags[i].name, "CVE"))<br>
+ printf(CVE_DB_URL "%s\n", tags[i].value);<br>
+ else if (!strcmp(tags[i].name, "linux-git"))<br>
+ printf(LINUX_GIT_URL "%s\n", tags[i].value);<br>
+ else<br>
+ printf("%s: %s\n", tags[i].name, tags[i].value);<br>
+ printf("\n");<br>
+ }<br>
+ }<br>
+}<br>
+<br>
static void check_option_collision(void)<br>
{<br>
unsigned int i, j;<br>
@@ -499,6 +526,7 @@ static void parse_opts(int argc, char *argv[])<br>
break;<br>
case 'h':<br>
print_help();<br>
+ print_test_info();<br>
exit(0);<br>
case 'i':<br>
iterations = atoi(optarg);<br>
@@ -584,26 +612,61 @@ int tst_parse_float(const char *str, float *val, float min, float max)<br>
return 0;<br>
}<br>
<br>
+static void print_colored(const char *str)<br>
+{<br>
+ if (tst_color_enabled(STDOUT_FILENO))<br>
+ printf("%s%s%s", ANSI_COLOR_YELLOW, str, ANSI_COLOR_RESET);<br>
+ else<br>
+ printf("%s", str);<br>
+}<br>
+<br>
+static void print_failure_hints(void)<br>
+{<br>
+ unsigned int i;<br>
+ const struct tst_tag *tags = tst_test->tags;<br>
+<br>
+ if (!tags)<br>
+ return;<br>
+<br>
+ for (i = 0; tags[i].name; i++) {<br>
+ if (!strcmp(tags[i].name, "linux-git")) {<br>
+ printf("\n");<br>
+ print_colored("HINT: ");</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ printf("This is a regression test for linux kernel, see commit:\n\n"<br>
+ LINUX_GIT_URL "%s\n", tags[i].value);<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">This sentence 'HINT: This is a ...' will be printed many times if there are many commits in tags, I prefer to see only once in front of these linux-kernel links.</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ }<br>
+<br>
+ if (!strcmp(tags[i].name, "CVE")) {<br>
+ printf("\n");<br>
+ print_colored("HINT: ");<br>
+ printf("This test also tests for CVE-%s, see:\n\n"<br>
+ CVE_DB_URL "%s\n", tags[i].value, tags[i].value);<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Here as well.</div></div><div><br></div></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>