[igt-dev] [PATCH i-g-t v2 0/2] runner: Unprintable characters in json

[igt-dev] [PATCH i-g-t v2 0/2] runner: Unprintable characters in json

[Petri Latvala]

Rebase of https://patchwork.freedesktop.org/series/70492/ adjusted for
handling dynamic subtest outputs. Patch 2/2 won't survive sending by
email, or patchwork's handling, but
https://gitlab.freedesktop.org/adrinael/igt-gpu-tools/commits/unprintable
has this version.

Petri Latvala (2):
  runner: Ensure generated json is properly UTF8-encoded
  runner/json_tests: Test handling of unprintable output from tests

 .../unprintable-characters/0/dmesg.txt        |   5 +
 .../unprintable-characters/0/err.txt          | 258 +++++++++++++++++
 .../unprintable-characters/0/journal.txt      |   2 +
 .../unprintable-characters/0/out.txt          | 259 ++++++++++++++++++
 .../unprintable-characters/README.txt         |   2 +
 .../unprintable-characters/endtime.txt        |   1 +
 .../unprintable-characters/joblist.txt        |   1 +
 .../unprintable-characters/metadata.txt       |  12 +
 .../unprintable-characters/reference.json     |  72 +++++
 .../unprintable-characters/starttime.txt      |   1 +
 .../unprintable-characters/uname.txt          |   1 +
 runner/resultgen.c                            |  45 ++-
 runner/runner_json_tests.c                    |   1 +
 13 files changed, 650 insertions(+), 10 deletions(-)
 create mode 100644 runner/json_tests_data/unprintable-characters/0/dmesg.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/0/err.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/0/journal.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/0/out.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/README.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/endtime.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/joblist.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/metadata.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/reference.json
 create mode 100644 runner/json_tests_data/unprintable-characters/starttime.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/uname.txt

-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v2 1/2] runner: Ensure generated json is properly UTF8-encoded

[Petri Latvala]

Sometimes tests output garbage (e.g. due to extreme occurrences of
https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/55) but we
need to present the garbage as results.

We already ignore any test output after the first \0, and for the rest
of the bytes that are not directly UTF-8 as-is, we can quite easily
represent them with two-byte UTF-8 encoding.

libjson-c already expects the string you feed it through
json_object_new_string* functions to be UTF-8.

v2: Rebase, adjust for dynamic subtest parsing

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> #v1
---
 runner/resultgen.c | 45 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/runner/resultgen.c b/runner/resultgen.c
index 2c8a55da..105ec887 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -405,15 +405,40 @@ static void free_matches(struct matches *matches)
 	free(matches->items);
 }
 
+static struct json_object *new_escaped_json_string(const char *buf, size_t len)
+{
+	struct json_object *obj;
+	char *str = NULL;
+	size_t strsize = 0;
+	size_t i;
+
+	for (i = 0; i < len; i++) {
+		if (buf[i] > 0 && buf[i] < 128) {
+			str = realloc(str, strsize + 1);
+			str[strsize] = buf[i];
+			++strsize;
+		} else {
+			/* Encode > 128 character to UTF-8. */
+			str = realloc(str, strsize + 2);
+			str[strsize] = ((unsigned char)buf[i] >> 6) | 0xC0;
+			str[strsize + 1] = ((unsigned char)buf[i] & 0x3F) | 0x80;
+			strsize += 2;
+		}
+	}
+
+	obj = json_object_new_string_len(str, strsize);
+	free(str);
+
+	return obj;
+}
+
 static void add_igt_version(struct json_object *testobj,
 			    const char *igt_version,
 			    size_t igt_version_len)
 {
 	if (igt_version)
 		json_object_object_add(testobj, "igt-version",
-				       json_object_new_string_len(igt_version,
-								  igt_version_len));
-
+				       new_escaped_json_string(igt_version, igt_version_len));
 }
 
 enum subtest_find_pattern {
@@ -608,7 +633,7 @@ static void process_dynamic_subtest_output(const char *piglit_name,
 		current_dynamic_test = get_or_create_json_object(tests, dynamic_piglit_name);
 
 		json_object_object_add(current_dynamic_test, key,
-				       json_object_new_string_len(dynbeg, dynend - dynbeg));
+				       new_escaped_json_string(dynbeg, dynend - dynbeg));
 		add_igt_version(current_dynamic_test, igt_version, igt_version_len);
 
 		if (!json_object_object_get_ex(current_dynamic_test, "result", NULL)) {
@@ -680,7 +705,7 @@ static bool fill_from_output(int fd, const char *binary, const char *key,
 		current_test = get_or_create_json_object(tests, piglit_name);
 
 		json_object_object_add(current_test, key,
-				       json_object_new_string_len(buf, statbuf.st_size));
+				       new_escaped_json_string(buf, statbuf.st_size));
 		add_igt_version(current_test, igt_version, igt_version_len);
 
 		return true;
@@ -704,7 +729,7 @@ static bool fill_from_output(int fd, const char *binary, const char *key,
 		end = find_subtest_end_limit(matches, begin_idx, result_idx, buf, bufend);
 
 		json_object_object_add(current_test, key,
-				       json_object_new_string_len(beg, end - beg));
+				       new_escaped_json_string(beg, end - beg));
 
 		add_igt_version(current_test, igt_version, igt_version_len);
 
@@ -863,11 +888,11 @@ static void add_dmesg(struct json_object *obj,
 		      const char *warnings, size_t warningslen)
 {
 	json_object_object_add(obj, "dmesg",
-			       json_object_new_string_len(dmesg, dmesglen));
+			       new_escaped_json_string(dmesg, dmesglen));
 
 	if (warnings) {
 		json_object_object_add(obj, "dmesg-warnings",
-				       json_object_new_string_len(warnings, warningslen));
+				       new_escaped_json_string(warnings, warningslen));
 	}
 }
 
@@ -1482,7 +1507,7 @@ struct json_object *generate_results_json(int dirfd)
 			r--;
 
 		json_object_object_add(obj, "uname",
-				       json_object_new_string_len(buf, r));
+				       new_escaped_json_string(buf, r));
 		close(fd);
 	}
 
@@ -1545,7 +1570,7 @@ struct json_object *generate_results_json(int dirfd)
 		s = read(fd, buf, sizeof(buf));
 
 		json_object_object_add(aborttest, "out",
-				       json_object_new_string_len(buf, s));
+				       new_escaped_json_string(buf, s));
 		json_object_object_add(aborttest, "err",
 				       json_object_new_string(""));
 		json_object_object_add(aborttest, "dmesg",
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v2 2/2] runner/json_tests: Test handling of unprintable output from tests

[Petri Latvala]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 16763 bytes --]

A simple test output with numbers from 1 to 255, both in plain text
form and as a single byte with that particular value.

Note that the json spec doesn't require \u-encoding for characters
other than '"', '\' and the range U+0000 to U+001F, the raw
non-\u-encoded UTF-8 in the reference.json file for bytes 128 and up
is what libjson-c outputs for those codepoints and is valid.

The validity of the json file can be verified with iconv, i.e.

 $ iconv -f UTF-8 reference.json -o /dev/null && echo it is utf-8

v2: Rebase over dynamic subtest tests, trivial

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> #v1
---
 .../unprintable-characters/0/dmesg.txt        |   5 +
 .../unprintable-characters/0/err.txt          | 258 +++++++++++++++++
 .../unprintable-characters/0/journal.txt      |   2 +
 .../unprintable-characters/0/out.txt          | 259 ++++++++++++++++++
 .../unprintable-characters/README.txt         |   2 +
 .../unprintable-characters/endtime.txt        |   1 +
 .../unprintable-characters/joblist.txt        |   1 +
 .../unprintable-characters/metadata.txt       |  12 +
 .../unprintable-characters/reference.json     |  72 +++++
 .../unprintable-characters/starttime.txt      |   1 +
 .../unprintable-characters/uname.txt          |   1 +
 runner/runner_json_tests.c                    |   1 +
 12 files changed, 615 insertions(+)
 create mode 100644 runner/json_tests_data/unprintable-characters/0/dmesg.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/0/err.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/0/journal.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/0/out.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/README.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/endtime.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/joblist.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/metadata.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/reference.json
 create mode 100644 runner/json_tests_data/unprintable-characters/starttime.txt
 create mode 100644 runner/json_tests_data/unprintable-characters/uname.txt

diff --git a/runner/json_tests_data/unprintable-characters/0/dmesg.txt b/runner/json_tests_data/unprintable-characters/0/dmesg.txt
new file mode 100644
index 00000000..6aac2735
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/0/dmesg.txt
@@ -0,0 +1,5 @@
+6,951,3216186095083,-;Console: switching to colour dummy device 80x25
+14,952,3216186095097,-;[IGT] successtest: executing
+14,953,3216186101115,-;[IGT] successtest: starting subtest first-subtest
+14,955,3216186101160,-;[IGT] successtest: exiting, ret=0
+6,956,3216186101299,-;Console: switching to colour frame buffer device 240x75
diff --git a/runner/json_tests_data/unprintable-characters/0/err.txt b/runner/json_tests_data/unprintable-characters/0/err.txt
new file mode 100644
index 00000000..34d19985
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/0/err.txt
@@ -0,0 +1,258 @@
+Starting subtest: first-subtest
+1:\x01
+2:\x02
+3:\x03
+4:\x04
+5:\x05
+6:\x06
+7:\a
+8:\b
+9:	
+10:
+
+11:\v
+12:\f
+13:
+14:\x0e
+15:\x0f
+16:\x10
+17:\x11
+18:\x12
+19:\x13
+20:\x14
+21:\x15
+22:\x16
+23:\x17
+24:\x18
+25:\x19
+26:\x1a
+27:^[
+28:\x1c
+29:\x1d
+30:\x1e
+31:\x1f
+32: 
+33:!
+34:"
+35:#
+36:$
+37:%
+38:&
+39:'
+40:(
+41:)
+42:*
+43:+
+44:,
+45:-
+46:.
+47:/
+48:0
+49:1
+50:2
+51:3
+52:4
+53:5
+54:6
+55:7
+56:8
+57:9
+58::
+59:;
+60:<
+61:=
+62:>
+63:?
+64:@
+65:A
+66:B
+67:C
+68:D
+69:E
+70:F
+71:G
+72:H
+73:I
+74:J
+75:K
+76:L
+77:M
+78:N
+79:O
+80:P
+81:Q
+82:R
+83:S
+84:T
+85:U
+86:V
+87:W
+88:X
+89:Y
+90:Z
+91:[
+92:\
+93:]
+94:^
+95:_
+96:`
+97:a
+98:b
+99:c
+100:d
+101:e
+102:f
+103:g
+104:h
+105:i
+106:j
+107:k
+108:l
+109:m
+110:n
+111:o
+112:p
+113:q
+114:r
+115:s
+116:t
+117:u
+118:v
+119:w
+120:x
+121:y
+122:z
+123:{
+124:|
+125:}
+126:~
+127:\x7f
+128:€
+129:
+130:‚
+131:ƒ
+132:„
+133:…
+134:†
+135:‡
+136:ˆ
+137:‰
+138:Š
+139:‹
+140:Œ
+141:
+142:Ž
+143:
+144:
+145:‘
+146:’
+147:“
+148:”
+149:•
+150:–
+151:—
+152:˜
+153:™
+154:š
+155:›
+156:œ
+157:
+158:ž
+159:Ÿ
+160: 
+161:¡
+162:¢
+163:£
+164:¤
+165:¥
+166:¦
+167:§
+168:¨
+169:©
+170:ª
+171:«
+172:¬
+173:­
+174:®
+175:¯
+176:°
+177:±
+178:²
+179:³
+180:´
+181:µ
+182:¶
+183:·
+184:¸
+185:¹
+186:º
+187:»
+188:¼
+189:½
+190:¾
+191:¿
+192:À
+193:Á
+194:Â
+195:Ã
+196:Ä
+197:Å
+198:Æ
+199:Ç
+200:È
+201:É
+202:Ê
+203:Ë
+204:Ì
+205:Í
+206:Î
+207:Ï
+208:Ð
+209:Ñ
+210:Ò
+211:Ó
+212:Ô
+213:Õ
+214:Ö
+215:×
+216:Ø
+217:Ù
+218:Ú
+219:Û
+220:Ü
+221:Ý
+222:Þ
+223:ß
+224:à
+225:á
+226:â
+227:ã
+228:ä
+229:å
+230:æ
+231:ç
+232:è
+233:é
+234:ê
+235:ë
+236:ì
+237:í
+238:î
+239:ï
+240:ð
+241:ñ
+242:ò
+243:ó
+244:ô
+245:õ
+246:ö
+247:÷
+248:ø
+249:ù
+250:ú
+251:û
+252:ü
+253:ý
+254:þ
+255:ÿ
+Subtest first-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/unprintable-characters/0/journal.txt b/runner/json_tests_data/unprintable-characters/0/journal.txt
new file mode 100644
index 00000000..86a30e07
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/0/journal.txt
@@ -0,0 +1,2 @@
+first-subtest
+exit:0 (0.014s)
diff --git a/runner/json_tests_data/unprintable-characters/0/out.txt b/runner/json_tests_data/unprintable-characters/0/out.txt
new file mode 100644
index 00000000..c597674a
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/0/out.txt
@@ -0,0 +1,259 @@
+IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)
+Starting subtest: first-subtest
+1:\x01
+2:\x02
+3:\x03
+4:\x04
+5:\x05
+6:\x06
+7:\a
+8:\b
+9:	
+10:
+
+11:\v
+12:\f
+13:
+14:\x0e
+15:\x0f
+16:\x10
+17:\x11
+18:\x12
+19:\x13
+20:\x14
+21:\x15
+22:\x16
+23:\x17
+24:\x18
+25:\x19
+26:\x1a
+27:^[
+28:\x1c
+29:\x1d
+30:\x1e
+31:\x1f
+32: 
+33:!
+34:"
+35:#
+36:$
+37:%
+38:&
+39:'
+40:(
+41:)
+42:*
+43:+
+44:,
+45:-
+46:.
+47:/
+48:0
+49:1
+50:2
+51:3
+52:4
+53:5
+54:6
+55:7
+56:8
+57:9
+58::
+59:;
+60:<
+61:=
+62:>
+63:?
+64:@
+65:A
+66:B
+67:C
+68:D
+69:E
+70:F
+71:G
+72:H
+73:I
+74:J
+75:K
+76:L
+77:M
+78:N
+79:O
+80:P
+81:Q
+82:R
+83:S
+84:T
+85:U
+86:V
+87:W
+88:X
+89:Y
+90:Z
+91:[
+92:\
+93:]
+94:^
+95:_
+96:`
+97:a
+98:b
+99:c
+100:d
+101:e
+102:f
+103:g
+104:h
+105:i
+106:j
+107:k
+108:l
+109:m
+110:n
+111:o
+112:p
+113:q
+114:r
+115:s
+116:t
+117:u
+118:v
+119:w
+120:x
+121:y
+122:z
+123:{
+124:|
+125:}
+126:~
+127:\x7f
+128:€
+129:
+130:‚
+131:ƒ
+132:„
+133:…
+134:†
+135:‡
+136:ˆ
+137:‰
+138:Š
+139:‹
+140:Œ
+141:
+142:Ž
+143:
+144:
+145:‘
+146:’
+147:“
+148:”
+149:•
+150:–
+151:—
+152:˜
+153:™
+154:š
+155:›
+156:œ
+157:
+158:ž
+159:Ÿ
+160: 
+161:¡
+162:¢
+163:£
+164:¤
+165:¥
+166:¦
+167:§
+168:¨
+169:©
+170:ª
+171:«
+172:¬
+173:­
+174:®
+175:¯
+176:°
+177:±
+178:²
+179:³
+180:´
+181:µ
+182:¶
+183:·
+184:¸
+185:¹
+186:º
+187:»
+188:¼
+189:½
+190:¾
+191:¿
+192:À
+193:Á
+194:Â
+195:Ã
+196:Ä
+197:Å
+198:Æ
+199:Ç
+200:È
+201:É
+202:Ê
+203:Ë
+204:Ì
+205:Í
+206:Î
+207:Ï
+208:Ð
+209:Ñ
+210:Ò
+211:Ó
+212:Ô
+213:Õ
+214:Ö
+215:×
+216:Ø
+217:Ù
+218:Ú
+219:Û
+220:Ü
+221:Ý
+222:Þ
+223:ß
+224:à
+225:á
+226:â
+227:ã
+228:ä
+229:å
+230:æ
+231:ç
+232:è
+233:é
+234:ê
+235:ë
+236:ì
+237:í
+238:î
+239:ï
+240:ð
+241:ñ
+242:ò
+243:ó
+244:ô
+245:õ
+246:ö
+247:÷
+248:ø
+249:ù
+250:ú
+251:û
+252:ü
+253:ý
+254:þ
+255:ÿ
+Subtest first-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/unprintable-characters/README.txt b/runner/json_tests_data/unprintable-characters/README.txt
new file mode 100644
index 00000000..046605ea
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/README.txt
@@ -0,0 +1,2 @@
+Any nonprintable output in stdout and stderr should yield a json file
+that is still valid UTF-8.
diff --git a/runner/json_tests_data/unprintable-characters/endtime.txt b/runner/json_tests_data/unprintable-characters/endtime.txt
new file mode 100644
index 00000000..635f6ae9
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/endtime.txt
@@ -0,0 +1 @@
+1539953735.172373
diff --git a/runner/json_tests_data/unprintable-characters/joblist.txt b/runner/json_tests_data/unprintable-characters/joblist.txt
new file mode 100644
index 00000000..81f914a7
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/joblist.txt
@@ -0,0 +1 @@
+successtest first-subtest
diff --git a/runner/json_tests_data/unprintable-characters/metadata.txt b/runner/json_tests_data/unprintable-characters/metadata.txt
new file mode 100644
index 00000000..c501ae0e
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/metadata.txt
@@ -0,0 +1,12 @@
+abort_mask : 0
+name : normal-run
+dry_run : 0
+sync : 0
+log_level : 0
+overwrite : 0
+multiple_mode : 0
+inactivity_timeout : 0
+use_watchdog : 0
+piglit_style_dmesg : 0
+test_root : /path/does/not/exist
+results_path : /path/does/not/exist
diff --git a/runner/json_tests_data/unprintable-characters/reference.json b/runner/json_tests_data/unprintable-characters/reference.json
new file mode 100644
index 00000000..d3add1eb
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/reference.json
@@ -0,0 +1,72 @@
+{
+  "__type__":"TestrunResult",
+  "results_version":10,
+  "name":"normal-run",
+  "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
+  "time_elapsed":{
+    "__type__":"TimeAttribute",
+    "start":1539953735.1110389,
+    "end":1539953735.1723731
+  },
+  "tests":{
+    "igt@successtest@first-subtest":{
+      "out":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)\nStarting subtest: first-subtest\n1:\u0001\n2:\u0002\n3:\u0003\n4:\u0004\n5:\u0005\n6:\u0006\n7:\u0007\n8:\b\n9:\t\n10:\n\n11:\u000b\n12:\f\n13:\r\n14:\u000e\n15:\u000f\n16:\u0010\n17:\u0011\n18:\u0012\n19:\u0013\n20:\u0014\n21:\u0015\n22:\u0016\n23:\u0017\n24:\u0018\n25:\u0019\n26:\u001a\n27:\u001b\n28:\u001c\n29:\u001d\n30:\u001e\n31:\u001f\n32: \n33:!\n34:\"\n35:#\n36:$\n37:%\n38:&\n39:'\n40:(\n41:)\n42:*\n43:+\n44:,\n45:-\n46:.\n47:\/\n48:0\n49:1\n50:2\n51:3\n52:4\n53:5\n54:6\n55:7\n56:8\n57:9\n58::\n59:;\n60:<\n61:=\n62:>\n63:?\n64:@\n65:A\n66:B\n67:C\n68:D\n69:E\n70:F\n71:G\n72:H\n73:I\n74:J\n75:K\n76:L\n77:M\n78:N\n79:O\n80:P\n81:Q\n82:R\n83:S\n84:T\n85:U\n86:V\n87:W\n88:X\n89:Y\n90:Z\n91:[\n92:\\\n93:]\n94:^\n95:_\n96:`\n97:a\n98:b\n99:c\n100:d\n101:e\n102:f\n103:g\n104:h\n105:i\n106:j\n107:k\n108:l\n109:m\n110:n\n111:o\n112:p\n113:q\n114:r\n115:s\n116:t\n117:u\n118:v\n119:w\n120:x\n121:y\n122:z\n123:{\n124:|\n125:}\n126:~\n127:\x7f\n128:€\n129:\n130:‚\n131:ƒ\n132:„\n133:…\n134:†\n135:‡\n136:ˆ\n137:‰\n138:Š\n139:‹\n140:Œ\n141:\n142:Ž\n143:\n144:\n145:‘\n146:’\n147:“\n148:”\n149:•\n150:–\n151:—\n152:˜\n153:™\n154:š\n155:›\n156:œ\n157:\n158:ž\n159:Ÿ\n160: \n161:¡\n162:¢\n163:£\n164:¤\n165:¥\n166:¦\n167:§\n168:¨\n169:©\n170:ª\n171:«\n172:¬\n173:­\n174:®\n175:¯\n176:°\n177:±\n178:²\n179:³\n180:´\n181:µ\n182:¶\n183:·\n184:¸\n185:¹\n186:º\n187:»\n188:¼\n189:½\n190:¾\n191:¿\n192:À\n193:Á\n194:Â\n195:Ã\n196:Ä\n197:Å\n198:Æ\n199:Ç\n200:È\n201:É\n202:Ê\n203:Ë\n204:Ì\n205:Í\n206:Î\n207:Ï\n208:Ð\n209:Ñ\n210:Ò\n211:Ó\n212:Ô\n213:Õ\n214:Ö\n215:×\n216:Ø\n217:Ù\n218:Ú\n219:Û\n220:Ü\n221:Ý\n222:Þ\n223:ß\n224:à\n225:á\n226:â\n227:ã\n228:ä\n229:å\n230:æ\n231:ç\n232:è\n233:é\n234:ê\n235:ë\n236:ì\n237:í\n238:î\n239:ï\n240:ð\n241:ñ\n242:ò\n243:ó\n244:ô\n245:õ\n246:ö\n247:÷\n248:ø\n249:ù\n250:ú\n251:û\n252:ü\n253:ý\n254:þ\n255:ÿ\nSubtest first-subtest: SUCCESS (0.000s)\n",
+      "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
+      "result":"warn",
+      "time":{
+        "__type__":"TimeAttribute",
+        "start":0,
+        "end":0
+      },
+      "err":"Starting subtest: first-subtest\n1:\u0001\n2:\u0002\n3:\u0003\n4:\u0004\n5:\u0005\n6:\u0006\n7:\u0007\n8:\b\n9:\t\n10:\n\n11:\u000b\n12:\f\n13:\r\n14:\u000e\n15:\u000f\n16:\u0010\n17:\u0011\n18:\u0012\n19:\u0013\n20:\u0014\n21:\u0015\n22:\u0016\n23:\u0017\n24:\u0018\n25:\u0019\n26:\u001a\n27:\u001b\n28:\u001c\n29:\u001d\n30:\u001e\n31:\u001f\n32: \n33:!\n34:\"\n35:#\n36:$\n37:%\n38:&\n39:'\n40:(\n41:)\n42:*\n43:+\n44:,\n45:-\n46:.\n47:\/\n48:0\n49:1\n50:2\n51:3\n52:4\n53:5\n54:6\n55:7\n56:8\n57:9\n58::\n59:;\n60:<\n61:=\n62:>\n63:?\n64:@\n65:A\n66:B\n67:C\n68:D\n69:E\n70:F\n71:G\n72:H\n73:I\n74:J\n75:K\n76:L\n77:M\n78:N\n79:O\n80:P\n81:Q\n82:R\n83:S\n84:T\n85:U\n86:V\n87:W\n88:X\n89:Y\n90:Z\n91:[\n92:\\\n93:]\n94:^\n95:_\n96:`\n97:a\n98:b\n99:c\n100:d\n101:e\n102:f\n103:g\n104:h\n105:i\n106:j\n107:k\n108:l\n109:m\n110:n\n111:o\n112:p\n113:q\n114:r\n115:s\n116:t\n117:u\n118:v\n119:w\n120:x\n121:y\n122:z\n123:{\n124:|\n125:}\n126:~\n127:\x7f\n128:€\n129:\n130:‚\n131:ƒ\n132:„\n133:…\n134:†\n135:‡\n136:ˆ\n137:‰\n138:Š\n139:‹\n140:Œ\n141:\n142:Ž\n143:\n144:\n145:‘\n146:’\n147:“\n148:”\n149:•\n150:–\n151:—\n152:˜\n153:™\n154:š\n155:›\n156:œ\n157:\n158:ž\n159:Ÿ\n160: \n161:¡\n162:¢\n163:£\n164:¤\n165:¥\n166:¦\n167:§\n168:¨\n169:©\n170:ª\n171:«\n172:¬\n173:­\n174:®\n175:¯\n176:°\n177:±\n178:²\n179:³\n180:´\n181:µ\n182:¶\n183:·\n184:¸\n185:¹\n186:º\n187:»\n188:¼\n189:½\n190:¾\n191:¿\n192:À\n193:Á\n194:Â\n195:Ã\n196:Ä\n197:Å\n198:Æ\n199:Ç\n200:È\n201:É\n202:Ê\n203:Ë\n204:Ì\n205:Í\n206:Î\n207:Ï\n208:Ð\n209:Ñ\n210:Ò\n211:Ó\n212:Ô\n213:Õ\n214:Ö\n215:×\n216:Ø\n217:Ù\n218:Ú\n219:Û\n220:Ü\n221:Ý\n222:Þ\n223:ß\n224:à\n225:á\n226:â\n227:ã\n228:ä\n229:å\n230:æ\n231:ç\n232:è\n233:é\n234:ê\n235:ë\n236:ì\n237:í\n238:î\n239:ï\n240:ð\n241:ñ\n242:ò\n243:ó\n244:ô\n245:õ\n246:ö\n247:÷\n248:ø\n249:ù\n250:ú\n251:û\n252:ü\n253:ý\n254:þ\n255:ÿ\nSubtest first-subtest: SUCCESS (0.000s)\n",
+      "dmesg":"<6> [3216186.095083] Console: switching to colour dummy device 80x25\n<6> [3216186.095097] [IGT] successtest: executing\n<6> [3216186.101115] [IGT] successtest: starting subtest first-subtest\n<6> [3216186.101160] [IGT] successtest: exiting, ret=0\n<6> [3216186.101299] Console: switching to colour frame buffer device 240x75\n"
+    }
+  },
+  "totals":{
+    "":{
+      "crash":0,
+      "pass":0,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":0,
+      "warn":1
+    },
+    "root":{
+      "crash":0,
+      "pass":0,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":0,
+      "warn":1
+    },
+    "igt@successtest":{
+      "crash":0,
+      "pass":0,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
+      "skip":0,
+      "incomplete":0,
+      "timeout":0,
+      "notrun":0,
+      "fail":0,
+      "warn":1
+    }
+  },
+  "runtimes":{
+    "igt@successtest":{
+      "time":{
+        "__type__":"TimeAttribute",
+        "start":0,
+        "end":0.014
+      }
+    }
+  }
+}
\ No newline at end of file
diff --git a/runner/json_tests_data/unprintable-characters/starttime.txt b/runner/json_tests_data/unprintable-characters/starttime.txt
new file mode 100644
index 00000000..ae038f18
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/starttime.txt
@@ -0,0 +1 @@
+1539953735.111039
diff --git a/runner/json_tests_data/unprintable-characters/uname.txt b/runner/json_tests_data/unprintable-characters/uname.txt
new file mode 100644
index 00000000..a7aef6f7
--- /dev/null
+++ b/runner/json_tests_data/unprintable-characters/uname.txt
@@ -0,0 +1 @@
+Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64
diff --git a/runner/runner_json_tests.c b/runner/runner_json_tests.c
index 21e87a02..bf4c285b 100644
--- a/runner/runner_json_tests.c
+++ b/runner/runner_json_tests.c
@@ -164,6 +164,7 @@ static const char *dirnames[] = {
 	"dmesg-warn-level-one-piglit-style",
 	"dynamic-subtests",
 	"dynamic-subtest-name-in-multiple-subtests",
+	"unprintable-characters",
 };
 
 igt_main
-- 
2.20.1


[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 1/2] runner: Ensure generated json is properly UTF8-encoded

[Arkadiusz Hiler]

On Fri, Jan 10, 2020 at 02:06:41PM +0200, Petri Latvala wrote:
> Sometimes tests output garbage (e.g. due to extreme occurrences of
> https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/55) but we
> need to present the garbage as results.
> 
> We already ignore any test output after the first \0, and for the rest
> of the bytes that are not directly UTF-8 as-is, we can quite easily
> represent them with two-byte UTF-8 encoding.
> 
> libjson-c already expects the string you feed it through
> json_object_new_string* functions to be UTF-8.
> 
> v2: Rebase, adjust for dynamic subtest parsing
> 
> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> #v1
> ---
>  runner/resultgen.c | 45 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index 2c8a55da..105ec887 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -405,15 +405,40 @@ static void free_matches(struct matches *matches)
>  	free(matches->items);
>  }
>  
> +static struct json_object *new_escaped_json_string(const char *buf, size_t len)
> +{
> +	struct json_object *obj;
> +	char *str = NULL;
> +	size_t strsize = 0;
> +	size_t i;
> +
> +	for (i = 0; i < len; i++) {
> +		if (buf[i] > 0 && buf[i] < 128) {
> +			str = realloc(str, strsize + 1);
> +			str[strsize] = buf[i];
> +			++strsize;
> +		} else {
> +			/* Encode > 128 character to UTF-8. */
> +			str = realloc(str, strsize + 2);
> +			str[strsize] = ((unsigned char)buf[i] >> 6) | 0xC0;
> +			str[strsize + 1] = ((unsigned char)buf[i] & 0x3F) | 0x80;
> +			strsize += 2;
> +		}
> +	}
> +
> +	obj = json_object_new_string_len(str, strsize);
> +	free(str);
> +
> +	return obj;
> +}

Looking at this for the 3rd time I wonder whether this realloc() every
character is not too costly, especially that we do that for every field.
Have you tried comparing times igt_results for some intermediates with
large dmesgs?

Anyway, let's not optimize prematurely:

Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

> +
>  static void add_igt_version(struct json_object *testobj,
>  			    const char *igt_version,
>  			    size_t igt_version_len)
>  {
>  	if (igt_version)
>  		json_object_object_add(testobj, "igt-version",
> -				       json_object_new_string_len(igt_version,
> -								  igt_version_len));
> -
> +				       new_escaped_json_string(igt_version, igt_version_len));
>  }
>  
>  enum subtest_find_pattern {
> @@ -608,7 +633,7 @@ static void process_dynamic_subtest_output(const char *piglit_name,
>  		current_dynamic_test = get_or_create_json_object(tests, dynamic_piglit_name);
>  
>  		json_object_object_add(current_dynamic_test, key,
> -				       json_object_new_string_len(dynbeg, dynend - dynbeg));
> +				       new_escaped_json_string(dynbeg, dynend - dynbeg));
>  		add_igt_version(current_dynamic_test, igt_version, igt_version_len);
>  
>  		if (!json_object_object_get_ex(current_dynamic_test, "result", NULL)) {
> @@ -680,7 +705,7 @@ static bool fill_from_output(int fd, const char *binary, const char *key,
>  		current_test = get_or_create_json_object(tests, piglit_name);
>  
>  		json_object_object_add(current_test, key,
> -				       json_object_new_string_len(buf, statbuf.st_size));
> +				       new_escaped_json_string(buf, statbuf.st_size));
>  		add_igt_version(current_test, igt_version, igt_version_len);
>  
>  		return true;
> @@ -704,7 +729,7 @@ static bool fill_from_output(int fd, const char *binary, const char *key,
>  		end = find_subtest_end_limit(matches, begin_idx, result_idx, buf, bufend);
>  
>  		json_object_object_add(current_test, key,
> -				       json_object_new_string_len(beg, end - beg));
> +				       new_escaped_json_string(beg, end - beg));
>  
>  		add_igt_version(current_test, igt_version, igt_version_len);
>  
> @@ -863,11 +888,11 @@ static void add_dmesg(struct json_object *obj,
>  		      const char *warnings, size_t warningslen)
>  {
>  	json_object_object_add(obj, "dmesg",
> -			       json_object_new_string_len(dmesg, dmesglen));
> +			       new_escaped_json_string(dmesg, dmesglen));
>  
>  	if (warnings) {
>  		json_object_object_add(obj, "dmesg-warnings",
> -				       json_object_new_string_len(warnings, warningslen));
> +				       new_escaped_json_string(warnings, warningslen));
>  	}
>  }
>  
> @@ -1482,7 +1507,7 @@ struct json_object *generate_results_json(int dirfd)
>  			r--;
>  
>  		json_object_object_add(obj, "uname",
> -				       json_object_new_string_len(buf, r));
> +				       new_escaped_json_string(buf, r));
>  		close(fd);
>  	}
>  
> @@ -1545,7 +1570,7 @@ struct json_object *generate_results_json(int dirfd)
>  		s = read(fd, buf, sizeof(buf));
>  
>  		json_object_object_add(aborttest, "out",
> -				       json_object_new_string_len(buf, s));
> +				       new_escaped_json_string(buf, s));
>  		json_object_object_add(aborttest, "err",
>  				       json_object_new_string(""));
>  		json_object_object_add(aborttest, "dmesg",
> -- 
> 2.20.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 2/2] runner/json_tests: Test handling of unprintable output from tests

[Arkadiusz Hiler]

On Fri, Jan 10, 2020 at 02:06:42PM +0200, Petri Latvala wrote:
> A simple test output with numbers from 1 to 255, both in plain text
> form and as a single byte with that particular value.
> 
> Note that the json spec doesn't require \u-encoding for characters
> other than '"', '\' and the range U+0000 to U+001F, the raw
> non-\u-encoded UTF-8 in the reference.json file for bytes 128 and up
> is what libjson-c outputs for those codepoints and is valid.
> 
> The validity of the json file can be verified with iconv, i.e.
> 
>  $ iconv -f UTF-8 reference.json -o /dev/null && echo it is utf-8
> 
> v2: Rebase over dynamic subtest tests, trivial
> 
> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> #v1

still looks good, and patchwork still does not like it... someday I'll
get to fixing the diff parsing logic and probably break other edge cases
in the process :-P

since gitlab's pipeline is passing feel free to merge it

Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 1/2] runner: Ensure generated json is properly UTF8-encoded

[Petri Latvala]

On Wed, Jan 15, 2020 at 12:29:57PM +0200, Arkadiusz Hiler wrote:
> On Fri, Jan 10, 2020 at 02:06:41PM +0200, Petri Latvala wrote:
> > Sometimes tests output garbage (e.g. due to extreme occurrences of
> > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/55) but we
> > need to present the garbage as results.
> > 
> > We already ignore any test output after the first \0, and for the rest
> > of the bytes that are not directly UTF-8 as-is, we can quite easily
> > represent them with two-byte UTF-8 encoding.
> > 
> > libjson-c already expects the string you feed it through
> > json_object_new_string* functions to be UTF-8.
> > 
> > v2: Rebase, adjust for dynamic subtest parsing
> > 
> > Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> #v1
> > ---
> >  runner/resultgen.c | 45 +++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 35 insertions(+), 10 deletions(-)
> > 
> > diff --git a/runner/resultgen.c b/runner/resultgen.c
> > index 2c8a55da..105ec887 100644
> > --- a/runner/resultgen.c
> > +++ b/runner/resultgen.c
> > @@ -405,15 +405,40 @@ static void free_matches(struct matches *matches)
> >  	free(matches->items);
> >  }
> >  
> > +static struct json_object *new_escaped_json_string(const char *buf, size_t len)
> > +{
> > +	struct json_object *obj;
> > +	char *str = NULL;
> > +	size_t strsize = 0;
> > +	size_t i;
> > +
> > +	for (i = 0; i < len; i++) {
> > +		if (buf[i] > 0 && buf[i] < 128) {
> > +			str = realloc(str, strsize + 1);
> > +			str[strsize] = buf[i];
> > +			++strsize;
> > +		} else {
> > +			/* Encode > 128 character to UTF-8. */
> > +			str = realloc(str, strsize + 2);
> > +			str[strsize] = ((unsigned char)buf[i] >> 6) | 0xC0;
> > +			str[strsize + 1] = ((unsigned char)buf[i] & 0x3F) | 0x80;
> > +			strsize += 2;
> > +		}
> > +	}
> > +
> > +	obj = json_object_new_string_len(str, strsize);
> > +	free(str);
> > +
> > +	return obj;
> > +}
> 
> Looking at this for the 3rd time I wonder whether this realloc() every
> character is not too costly, especially that we do that for every field.

Do you mean as opposed to allocating a larger chunk at a time? realloc
already does this.

With a quick whipup test, realloc()ing same pointer repeatedly for
sizes 1 to 0xffffff (randomly chosen end point) with increments of 1,
the returned pointer was different a total of 29 times. For funzies, a
total of 9 times when stdout was a pipe instead of tty.

> Have you tried comparing times igt_results for some intermediates with
> large dmesgs?

I can do that but I won't be expecting much difference.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 1/2] runner: Ensure generated json is properly UTF8-encoded

[Arkadiusz Hiler]

On Wed, Jan 15, 2020 at 12:48:47PM +0200, Petri Latvala wrote:
> On Wed, Jan 15, 2020 at 12:29:57PM +0200, Arkadiusz Hiler wrote:
> > On Fri, Jan 10, 2020 at 02:06:41PM +0200, Petri Latvala wrote:
> > > Sometimes tests output garbage (e.g. due to extreme occurrences of
> > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/55) but we
> > > need to present the garbage as results.
> > > 
> > > We already ignore any test output after the first \0, and for the rest
> > > of the bytes that are not directly UTF-8 as-is, we can quite easily
> > > represent them with two-byte UTF-8 encoding.
> > > 
> > > libjson-c already expects the string you feed it through
> > > json_object_new_string* functions to be UTF-8.
> > > 
> > > v2: Rebase, adjust for dynamic subtest parsing
> > > 
> > > Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> #v1
> > > ---
> > >  runner/resultgen.c | 45 +++++++++++++++++++++++++++++++++++----------
> > >  1 file changed, 35 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/runner/resultgen.c b/runner/resultgen.c
> > > index 2c8a55da..105ec887 100644
> > > --- a/runner/resultgen.c
> > > +++ b/runner/resultgen.c
> > > @@ -405,15 +405,40 @@ static void free_matches(struct matches *matches)
> > >  	free(matches->items);
> > >  }
> > >  
> > > +static struct json_object *new_escaped_json_string(const char *buf, size_t len)
> > > +{
> > > +	struct json_object *obj;
> > > +	char *str = NULL;
> > > +	size_t strsize = 0;
> > > +	size_t i;
> > > +
> > > +	for (i = 0; i < len; i++) {
> > > +		if (buf[i] > 0 && buf[i] < 128) {
> > > +			str = realloc(str, strsize + 1);
> > > +			str[strsize] = buf[i];
> > > +			++strsize;
> > > +		} else {
> > > +			/* Encode > 128 character to UTF-8. */
> > > +			str = realloc(str, strsize + 2);
> > > +			str[strsize] = ((unsigned char)buf[i] >> 6) | 0xC0;
> > > +			str[strsize + 1] = ((unsigned char)buf[i] & 0x3F) | 0x80;
> > > +			strsize += 2;
> > > +		}
> > > +	}
> > > +
> > > +	obj = json_object_new_string_len(str, strsize);
> > > +	free(str);
> > > +
> > > +	return obj;
> > > +}
> > 
> > Looking at this for the 3rd time I wonder whether this realloc() every
> > character is not too costly, especially that we do that for every field.
> 
> Do you mean as opposed to allocating a larger chunk at a time? realloc
> already does this.
> 
> With a quick whipup test, realloc()ing same pointer repeatedly for
> sizes 1 to 0xffffff (randomly chosen end point) with increments of 1,
> the returned pointer was different a total of 29 times. For funzies, a
> total of 9 times when stdout was a pipe instead of tty.

I have expected that even if it's not a part of the standard the actual
implementation would optimize for this. Nice to see it's this good.

> > Have you tried comparing times igt_results for some intermediates with
> > large dmesgs?
> 
> I can do that but I won't be expecting much difference.

Yeah, does not make sense to test it. Go ahead with mergeing.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v2 1/2] runner: Ensure generated json is properly UTF8-encoded

[Petri Latvala]

On Wed, Jan 15, 2020 at 12:54:56PM +0200, Arkadiusz Hiler wrote:
> On Wed, Jan 15, 2020 at 12:48:47PM +0200, Petri Latvala wrote:
> > On Wed, Jan 15, 2020 at 12:29:57PM +0200, Arkadiusz Hiler wrote:
> > > On Fri, Jan 10, 2020 at 02:06:41PM +0200, Petri Latvala wrote:
> > > > Sometimes tests output garbage (e.g. due to extreme occurrences of
> > > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/55) but we
> > > > need to present the garbage as results.
> > > > 
> > > > We already ignore any test output after the first \0, and for the rest
> > > > of the bytes that are not directly UTF-8 as-is, we can quite easily
> > > > represent them with two-byte UTF-8 encoding.
> > > > 
> > > > libjson-c already expects the string you feed it through
> > > > json_object_new_string* functions to be UTF-8.
> > > > 
> > > > v2: Rebase, adjust for dynamic subtest parsing
> > > > 
> > > > Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> #v1
> > > > ---
> > > >  runner/resultgen.c | 45 +++++++++++++++++++++++++++++++++++----------
> > > >  1 file changed, 35 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/runner/resultgen.c b/runner/resultgen.c
> > > > index 2c8a55da..105ec887 100644
> > > > --- a/runner/resultgen.c
> > > > +++ b/runner/resultgen.c
> > > > @@ -405,15 +405,40 @@ static void free_matches(struct matches *matches)
> > > >  	free(matches->items);
> > > >  }
> > > >  
> > > > +static struct json_object *new_escaped_json_string(const char *buf, size_t len)
> > > > +{
> > > > +	struct json_object *obj;
> > > > +	char *str = NULL;
> > > > +	size_t strsize = 0;
> > > > +	size_t i;
> > > > +
> > > > +	for (i = 0; i < len; i++) {
> > > > +		if (buf[i] > 0 && buf[i] < 128) {
> > > > +			str = realloc(str, strsize + 1);
> > > > +			str[strsize] = buf[i];
> > > > +			++strsize;
> > > > +		} else {
> > > > +			/* Encode > 128 character to UTF-8. */
> > > > +			str = realloc(str, strsize + 2);
> > > > +			str[strsize] = ((unsigned char)buf[i] >> 6) | 0xC0;
> > > > +			str[strsize + 1] = ((unsigned char)buf[i] & 0x3F) | 0x80;
> > > > +			strsize += 2;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	obj = json_object_new_string_len(str, strsize);
> > > > +	free(str);
> > > > +
> > > > +	return obj;
> > > > +}
> > > 
> > > Looking at this for the 3rd time I wonder whether this realloc() every
> > > character is not too costly, especially that we do that for every field.
> > 
> > Do you mean as opposed to allocating a larger chunk at a time? realloc
> > already does this.
> > 
> > With a quick whipup test, realloc()ing same pointer repeatedly for
> > sizes 1 to 0xffffff (randomly chosen end point) with increments of 1,
> > the returned pointer was different a total of 29 times. For funzies, a
> > total of 9 times when stdout was a pipe instead of tty.
> 
> I have expected that even if it's not a part of the standard the actual
> implementation would optimize for this. Nice to see it's this good.
> 
> > > Have you tried comparing times igt_results for some intermediates with
> > > large dmesgs?
> > 
> > I can do that but I won't be expecting much difference.
> 
> Yeah, does not make sense to test it. Go ahead with mergeing.

Thanks. Merged now.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev