Patchwork hmp: hmp_memchar_read(): skip non-printable chars

login
register
mail settings
Submitter Luiz Capitulino
Date Jan. 29, 2013, 12:35 p.m.
Message ID <20130129103535.13c23f71@doriath.home>
Download mbox | patch
Permalink /patch/216513/
State New
Headers show

Comments

Luiz Capitulino - Jan. 29, 2013, 12:35 p.m.
Otherwise we can get funny stuff printed and if the buffer contains
a '\0' char it won't be fully printed.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
Markus Armbruster - Jan. 29, 2013, 1:20 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> Otherwise we can get funny stuff printed and if the buffer contains
> a '\0' char it won't be fully printed.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  hmp.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 249b89b..5bfc8bd 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -679,8 +679,10 @@ void hmp_memchar_read(Monitor *mon, const QDict *qdict)
>  {
>      uint32_t size = qdict_get_int(qdict, "size");
>      const char *chardev = qdict_get_str(qdict, "device");
> +    bool print_nl = false;
>      MemCharRead *meminfo;
>      Error *errp = NULL;
> +    int i;
>  
>      meminfo = qmp_memchar_read(chardev, size, false, 0, &errp);
>      if (errp) {
> @@ -689,8 +691,16 @@ void hmp_memchar_read(Monitor *mon, const QDict *qdict)
>          return;
>      }
>  
> -    if (meminfo->count > 0) {
> -        monitor_printf(mon, "%s\n", meminfo->data);
> +    for (i = 0; i < meminfo->count; i++) {
> +        char c = meminfo->data[i];
> +        if (isprint(c) || c == '\n') {
> +            monitor_printf(mon, "%c", c);
> +            print_nl = true;
> +        }
> +    }
> +
> +    if (print_nl) {
> +        monitor_printf(mon, "\n");
>      }
>  
>      qapi_free_MemCharRead(meminfo);

Swallows control characters except for '\n'.  Okay if that's what we
want, but clearly needs to be documented.

If not, then something like this:

   for (i = 0; i < meminfo->count; i++) {
       char c = meminfo->data[i];

        if (c == '\') {
            monitor_printf(mon, "\\\\");
        } else if (isprint(c) || c == '\n' || c == '\t') {
            monitor_printf(mon, "%c, c);
        } else {
            monitor_printf(mon, "\\%03o", c);
        }
    }
    if (meminfo->count) {
        monitor_printf(mon, "\n");
    }

Also needs documenting.
Luiz Capitulino - Jan. 29, 2013, 1:39 p.m.
On Tue, 29 Jan 2013 14:20:30 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Otherwise we can get funny stuff printed and if the buffer contains
> > a '\0' char it won't be fully printed.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hmp.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index 249b89b..5bfc8bd 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -679,8 +679,10 @@ void hmp_memchar_read(Monitor *mon, const QDict *qdict)
> >  {
> >      uint32_t size = qdict_get_int(qdict, "size");
> >      const char *chardev = qdict_get_str(qdict, "device");
> > +    bool print_nl = false;
> >      MemCharRead *meminfo;
> >      Error *errp = NULL;
> > +    int i;
> >  
> >      meminfo = qmp_memchar_read(chardev, size, false, 0, &errp);
> >      if (errp) {
> > @@ -689,8 +691,16 @@ void hmp_memchar_read(Monitor *mon, const QDict *qdict)
> >          return;
> >      }
> >  
> > -    if (meminfo->count > 0) {
> > -        monitor_printf(mon, "%s\n", meminfo->data);
> > +    for (i = 0; i < meminfo->count; i++) {
> > +        char c = meminfo->data[i];
> > +        if (isprint(c) || c == '\n') {
> > +            monitor_printf(mon, "%c", c);
> > +            print_nl = true;
> > +        }
> > +    }
> > +
> > +    if (print_nl) {
> > +        monitor_printf(mon, "\n");
> >      }
> >  
> >      qapi_free_MemCharRead(meminfo);
> 
> Swallows control characters except for '\n'.  Okay if that's what we
> want, but clearly needs to be documented.

Well, the goal here is to be as friendly as possible when printing
this stuff to users. You do improve it, so I'll merge it into my
patch. Except that, I don't see what has to be documented, as documentation
goes to hmp's help message.

Patch

diff --git a/hmp.c b/hmp.c
index 249b89b..5bfc8bd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -679,8 +679,10 @@  void hmp_memchar_read(Monitor *mon, const QDict *qdict)
 {
     uint32_t size = qdict_get_int(qdict, "size");
     const char *chardev = qdict_get_str(qdict, "device");
+    bool print_nl = false;
     MemCharRead *meminfo;
     Error *errp = NULL;
+    int i;
 
     meminfo = qmp_memchar_read(chardev, size, false, 0, &errp);
     if (errp) {
@@ -689,8 +691,16 @@  void hmp_memchar_read(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    if (meminfo->count > 0) {
-        monitor_printf(mon, "%s\n", meminfo->data);
+    for (i = 0; i < meminfo->count; i++) {
+        char c = meminfo->data[i];
+        if (isprint(c) || c == '\n') {
+            monitor_printf(mon, "%c", c);
+            print_nl = true;
+        }
+    }
+
+    if (print_nl) {
+        monitor_printf(mon, "\n");
     }
 
     qapi_free_MemCharRead(meminfo);