diff mbox

hmp: hmp_memchar_read(): skip non-printable chars

Message ID 20130129103535.13c23f71@doriath.home
State New
Headers show

Commit Message

Luiz Capitulino Jan. 29, 2013, 12:35 p.m. UTC
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(-)

Comments

Markus Armbruster Jan. 29, 2013, 1:20 p.m. UTC | #1
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. UTC | #2
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.
diff mbox

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);