[ovs-dev,v2,1/2] table: append newline when printing tables

Message ID 20180808003452.21396-1-aconole@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev,v2,1/2] table: append newline when printing tables
Related show

Commit Message

Aaron Conole Aug. 8, 2018, 12:34 a.m.
With commit cb139fa8b3a1 ("table: New function table_format() for
formatting a table as a string.") a new mechanism for formatting
tables was introduced, and the table_print method was refactored to
use this.

During that refactor, calls to 'puts' were replaced with
'ds_put_cstr', and table print was changed to use 'fputs(...,
stdout)'.  Unfortunately, fputs() does not append a newline to the
string provided, and changes the output strings of, for example,
ovsdb-client dump to print all on one line.  This means
post-processing scripts that are chained after ovsdb-client would
either block indefinitely (if they don't detect EOF), or process the
entire bundle at once (rather than seeing each table on a separate
line).

Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table as a string.")
Cc: Ben Pfaff <blp@ovn.org>
Cc: Jakub Sitnicki <jsitnicki@gmail.com>
Reported-by: Terry Wilson <twilson@redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
Signed-off-by: Aaron Conole <aconole@redhat.com>
Suggested-by: Ben Pfaff <blp@ovn.org>
---
 lib/table.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Terry Wilson Aug. 8, 2018, 3:15 p.m. | #1
On Tue, Aug 7, 2018 at 7:34 PM, Aaron Conole <aconole@redhat.com> wrote:
> With commit cb139fa8b3a1 ("table: New function table_format() for
> formatting a table as a string.") a new mechanism for formatting
> tables was introduced, and the table_print method was refactored to
> use this.
>
> During that refactor, calls to 'puts' were replaced with
> 'ds_put_cstr', and table print was changed to use 'fputs(...,
> stdout)'.  Unfortunately, fputs() does not append a newline to the
> string provided, and changes the output strings of, for example,
> ovsdb-client dump to print all on one line.  This means
> post-processing scripts that are chained after ovsdb-client would
> either block indefinitely (if they don't detect EOF), or process the
> entire bundle at once (rather than seeing each table on a separate
> line).
>
> Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table as a string.")
> Cc: Ben Pfaff <blp@ovn.org>
> Cc: Jakub Sitnicki <jsitnicki@gmail.com>
> Reported-by: Terry Wilson <twilson@redhat.com>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> Suggested-by: Ben Pfaff <blp@ovn.org>
> ---
>  lib/table.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/table.c b/lib/table.c
> index cd811caf5..19bf89262 100644
> --- a/lib/table.c
> +++ b/lib/table.c
> @@ -547,6 +547,7 @@ table_print_json__(const struct table *table, const struct table_style *style,
>      json_object_put(json, "data", data);
>
>      json_to_ds(json, style->json_flags, s);
> +    ds_put_char(s, '\n');
>      json_destroy(json);
>  }
>
> --
> 2.14.3
>

I can verify that this re-adds the newline and that it fixes the issue
in neutron's ovs-agent.

Acked-by: Terry Wilson <twilson@redhat.com>
Tested-by: Terry Wilson <twilson@redhat.com>
Ben Pfaff Aug. 8, 2018, 6:17 p.m. | #2
On Wed, Aug 08, 2018 at 10:15:35AM -0500, Terry Wilson wrote:
> On Tue, Aug 7, 2018 at 7:34 PM, Aaron Conole <aconole@redhat.com> wrote:
> > With commit cb139fa8b3a1 ("table: New function table_format() for
> > formatting a table as a string.") a new mechanism for formatting
> > tables was introduced, and the table_print method was refactored to
> > use this.
> >
> > During that refactor, calls to 'puts' were replaced with
> > 'ds_put_cstr', and table print was changed to use 'fputs(...,
> > stdout)'.  Unfortunately, fputs() does not append a newline to the
> > string provided, and changes the output strings of, for example,
> > ovsdb-client dump to print all on one line.  This means
> > post-processing scripts that are chained after ovsdb-client would
> > either block indefinitely (if they don't detect EOF), or process the
> > entire bundle at once (rather than seeing each table on a separate
> > line).
> >
> > Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table as a string.")
> > Cc: Ben Pfaff <blp@ovn.org>
> > Cc: Jakub Sitnicki <jsitnicki@gmail.com>
> > Reported-by: Terry Wilson <twilson@redhat.com>
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
> > Signed-off-by: Aaron Conole <aconole@redhat.com>
> > Suggested-by: Ben Pfaff <blp@ovn.org>
> > ---
> >  lib/table.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/table.c b/lib/table.c
> > index cd811caf5..19bf89262 100644
> > --- a/lib/table.c
> > +++ b/lib/table.c
> > @@ -547,6 +547,7 @@ table_print_json__(const struct table *table, const struct table_style *style,
> >      json_object_put(json, "data", data);
> >
> >      json_to_ds(json, style->json_flags, s);
> > +    ds_put_char(s, '\n');
> >      json_destroy(json);
> >  }
> >
> > --
> > 2.14.3
> >
> 
> I can verify that this re-adds the newline and that it fixes the issue
> in neutron's ovs-agent.
> 
> Acked-by: Terry Wilson <twilson@redhat.com>
> Tested-by: Terry Wilson <twilson@redhat.com>

This now has the world's longest collection of tags for a one-line
change:

Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table as a string.")
Cc: Ben Pfaff <blp@ovn.org>
Cc: Jakub Sitnicki <jsitnicki@gmail.com>
Reported-by: Terry Wilson <twilson@redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
Signed-off-by: Aaron Conole <aconole@redhat.com>
Suggested-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Terry Wilson <twilson@redhat.com>
Tested-by: Terry Wilson <twilson@redhat.com>

I applied this to master and branch-2.10.

Patch

diff --git a/lib/table.c b/lib/table.c
index cd811caf5..19bf89262 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -547,6 +547,7 @@  table_print_json__(const struct table *table, const struct table_style *style,
     json_object_put(json, "data", data);
 
     json_to_ds(json, style->json_flags, s);
+    ds_put_char(s, '\n');
     json_destroy(json);
 }