[Qemu-devel] [PATCH v4 02/14] qapi: Move to_c_string() to common.py

Max Reitz posted 14 patches 6 years, 7 months ago
Maintainers: "Denis V. Lunev" <den@openvz.org>, Liu Yuan <namei.unix@gmail.com>, Jeff Cody <codyprime@gmail.com>, Kevin Wolf <kwolf@redhat.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Markus Armbruster <armbru@redhat.com>, Eric Blake <eblake@redhat.com>, Stefan Weil <sw@weilnetz.de>, Max Reitz <mreitz@redhat.com>
[Qemu-devel] [PATCH v4 02/14] qapi: Move to_c_string() to common.py
Posted by Max Reitz 6 years, 7 months ago
This function will be useful for code generation once we allow default
values, so move it to the other "C helper functions".  In the process,
rewrite it so it supports all nonprintable and non-ASCII characters.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 scripts/qapi/common.py     | 26 ++++++++++++++++++++++++++
 scripts/qapi/introspect.py |  4 ----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 3396ea4a09..c6754a5856 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -2208,6 +2208,32 @@ def c_fname(filename):
     return re.sub(r'[^A-Za-z0-9_]', '_', filename)
 
 
+# Translates a string to a valid C constant
+def to_c_string(string):
+    result = '"'
+
+    python2 = isinstance(string, bytes)
+    if not python2:
+        # Will return integers when iterated over
+        string = string.encode()
+
+    for c in string:
+        value = ord(c) if python2 else c
+        if value < 0x20 or value > 0x7e:
+            result += '\\%03o' % value
+        else:
+            c = chr(value)
+            if c == '"':
+                result += '\\"'
+            elif c == '\\':
+                result += '\\\\'
+            else:
+                result += c
+
+    result += '"'
+    return result
+
+
 def guardstart(name):
     return mcgen('''
 #ifndef %(name)s
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 6a61dd831f..572e0b8331 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -66,10 +66,6 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
     return ret
 
 
-def to_c_string(string):
-    return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
-
-
 class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
 
     def __init__(self, prefix, unmask):
-- 
2.21.0


Re: [Qemu-devel] [PATCH v4 02/14] qapi: Move to_c_string() to common.py
Posted by Markus Armbruster 6 years, 2 months ago
Max Reitz <mreitz@redhat.com> writes:

> This function will be useful for code generation once we allow default
> values, so move it to the other "C helper functions".  In the process,
> rewrite it so it supports all nonprintable and non-ASCII characters.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Please have a close look at commit 56a8caff92 "qapi: Restrict strings to
printable ASCII".  Do we still need the rewrite?

If yes: the commit message title promises code motion, but the patch is
anything but.  Adjust the title, please.


Re: [Qemu-devel] [PATCH v4 02/14] qapi: Move to_c_string() to common.py
Posted by Max Reitz 6 years, 2 months ago
On 14.11.19 10:20, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> writes:
> 
>> This function will be useful for code generation once we allow default
>> values, so move it to the other "C helper functions".  In the process,
>> rewrite it so it supports all nonprintable and non-ASCII characters.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Please have a close look at commit 56a8caff92 "qapi: Restrict strings to
> printable ASCII".  Do we still need the rewrite?

If that’s all that has changed, I think we will still need at least some
bits, like the " or \ escaping.

Also, actually, it looks like 56a8caff92 didn’t change the fact that
control characters are verbatim parts of the string, i.e. \u000a will
still be a literal 0xa byte, and as such must be escaped anew in the C
string.

So without having tried, I think this is still very much necessary.

> If yes: the commit message title promises code motion, but the patch is
> anything but.  Adjust the title, please.

OK.

Max