[LTP] [PATCH 1/1] Docparse: Escape | in table content

[LTP] [PATCH 1/1] Docparse: Escape | in table content

[Petr Vorel]

31708e815 added | in values displayed in table, which unescaped breaks
table formatting.

NOTE: Due asciidoc{,tor} markup design it's difficult to easily escape markup
handling only this particular case.

https://github.com/asciidoctor/asciidoctor/issues/901
https://discuss.asciidoctor.org/Difficulty-in-consistently-escaping-markup-td5318.html

Fixes: #754

Reported-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 docparse/testinfo.pl | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/docparse/testinfo.pl b/docparse/testinfo.pl
index d8d9ea663..5aa7315e4 100755
--- a/docparse/testinfo.pl
+++ b/docparse/testinfo.pl
@@ -119,6 +119,14 @@ sub table
 	return "|===\n";
 }
 
+sub table_escape
+{
+	my $out = $_[0];
+
+	$out =~ s/\|/\\|/g;
+	return $out;
+}
+
 sub print_defined
 {
 	my ($key, $val, $val2) = @_;
@@ -329,10 +337,11 @@ sub content_all_tests
 
 			$content .= "|" . tag2title($k) . "\n|";
 			if (ref($v) eq 'ARRAY') {
-				$content .= join(', ', @$v),
+				$content .= table_escape(join(', ', @$v));
 			} else {
-				$content .= $v;
+				$content .= table_escape($v);
 			}
+
 			$content .= "\n";
 
 			$tmp2 = 1;
-- 
2.29.2

[LTP] [PATCH 1/1] Docparse: Escape | in table content

[Cyril Hrubis]

Hi!
Looks good to me, acked.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/1] Docparse: Escape | in table content

[Petr Vorel]

Hi Cyril,

> Hi!
> Looks good to me, acked.
thanks for your review, merged!

BTW I wonder how to fix .test_variants in JSON:
"test_variants": "ARRAY_SIZE"

It's in many tests, e.g.  testcases/kernel/syscalls/clock_gettime/clock_gettime02.c.

Also many arrays should be anonymous, e.g. exe_options (for .options) and
resource_files (for .resource_files) in testcases/kernel/syscalls/execve/execve05.c.
I wonder if there can be smarter solution before we fix it.

But using inline for .options (and other two dimensional array needs to be fixed
again in the parser: it now prints array reference: ARRAY(0x563186f58048)
(for testcases/kernel/syscalls/ioctl/ioctl01.c).

Because this disables reusing things (putting them in the header). But you
probably prefer it than slower down parsing with preprocessor.
But we need to state this policy in docs.

Kind regards,
Petr

[LTP] [PATCH 1/1] Docparse: Escape | in table content

[Cyril Hrubis]

Hi!
> BTW I wonder how to fix .test_variants in JSON:
> "test_variants": "ARRAY_SIZE"
> 
> It's in many tests, e.g.  testcases/kernel/syscalls/clock_gettime/clock_gettime02.c.

I guess that we can filter out test_variants for now, I do not think
that it will be useful for the metadata that much.

> Also many arrays should be anonymous, e.g. exe_options (for .options) and
> resource_files (for .resource_files) in testcases/kernel/syscalls/execve/execve05.c.
> I wonder if there can be smarter solution before we fix it.

Well either we teach the C parser about variables, or we move all the
definitions inline. I would just move them since that's easier to do.

> But using inline for .options (and other two dimensional array needs to be fixed
> again in the parser: it now prints array reference: ARRAY(0x563186f58048)
> (for testcases/kernel/syscalls/ioctl/ioctl01.c).

That's a bug in the perl that produces the asciidoc, it's parsed just
fine in the json:

  "ioctl01": {
   "needs_root": "1",
   "needs_tmpdir": "1",
   "options": [
     [
      "D:",
      "device",
      "-D <tty device> : for example, /dev/tty[0-9]"
     ]
    ],
   "fname": "testcases/kernel/syscalls/ioctl/ioctl01.c"
  },

> Because this disables reusing things (putting them in the header). But you
> probably prefer it than slower down parsing with preprocessor.
> But we need to state this policy in docs.

Indeed, that should be done.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/1] Docparse: Escape | in table content

[Petr Vorel]

> Hi!
> > BTW I wonder how to fix .test_variants in JSON:
> > "test_variants": "ARRAY_SIZE"

> > It's in many tests, e.g.  testcases/kernel/syscalls/clock_gettime/clock_gettime02.c.

> I guess that we can filter out test_variants for now, I do not think
> that it will be useful for the metadata that much.

+1, I'll do it.

> > Also many arrays should be anonymous, e.g. exe_options (for .options) and
> > resource_files (for .resource_files) in testcases/kernel/syscalls/execve/execve05.c.
> > I wonder if there can be smarter solution before we fix it.

> Well either we teach the C parser about variables, or we move all the
> definitions inline. I would just move them since that's easier to do.

+1.

> > But using inline for .options (and other two dimensional array needs to be fixed
> > again in the parser: it now prints array reference: ARRAY(0x563186f58048)
> > (for testcases/kernel/syscalls/ioctl/ioctl01.c).

> That's a bug in the perl that produces the asciidoc, it's parsed just
> fine in the json:

Sure, I meant perl parser, which is actually formatter.
I'll try to send a patch soon.

>   "ioctl01": {
>    "needs_root": "1",
>    "needs_tmpdir": "1",
>    "options": [
>      [
>       "D:",
>       "device",
>       "-D <tty device> : for example, /dev/tty[0-9]"
>      ]
>     ],
>    "fname": "testcases/kernel/syscalls/ioctl/ioctl01.c"
>   },

> > Because this disables reusing things (putting them in the header). But you
> > probably prefer it than slower down parsing with preprocessor.
> > But we need to state this policy in docs.

> Indeed, that should be done.


Kind regards,
Petr

[LTP] [PATCH 1/1] Docparse: Escape | in table content

[Petr Vorel]

> > Hi!
> > > BTW I wonder how to fix .test_variants in JSON:
> > > "test_variants": "ARRAY_SIZE"

> > > It's in many tests, e.g.  testcases/kernel/syscalls/clock_gettime/clock_gettime02.c.

> > I guess that we can filter out test_variants for now, I do not think
> > that it will be useful for the metadata that much.

> +1, I'll do it.

Actually, thinking about it twice I'd quite like to have all tags.
Here test variant is for me info that test either test both raw syscall and libc
wrapper or these tests have support for time64 (Viresh Kumar's patches).

So for me would be enough boolean info, not the actual number of variants.

Kind regards,
Petr

[LTP] [PATCH 1/1] Docparse: Escape | in table content

[Petr Vorel]

Hi Cyril,

...
> > Also many arrays should be anonymous, e.g. exe_options (for .options) and
> > resource_files (for .resource_files) in testcases/kernel/syscalls/execve/execve05.c.
> > I wonder if there can be smarter solution before we fix it.

> Well either we teach the C parser about variables, or we move all the
> definitions inline. I would just move them since that's easier to do.
I've fixed most of .options and .save_restore.
There are still some (e.g. CORE_PATTERN in madvise08 - use macro).

> > But using inline for .options (and other two dimensional array needs to be fixed
> > again in the parser: it now prints array reference: ARRAY(0x563186f58048)
> > (for testcases/kernel/syscalls/ioctl/ioctl01.c).

> That's a bug in the perl that produces the asciidoc, it's parsed just
> fine in the json:

>   "ioctl01": {
>    "needs_root": "1",
>    "needs_tmpdir": "1",
>    "options": [
>      [
>       "D:",
>       "device",
>       "-D <tty device> : for example, /dev/tty[0-9]"
>      ]
>     ],
>    "fname": "testcases/kernel/syscalls/ioctl/ioctl01.c"
>   },

I've fixed this in dca56c567.

Kind regards,
Petr