[ovs-dev] table: append newline when printing tables

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>
---
NOTE: I chose to keep the fputs and insert an additional fputs.
      It might alternately be appropriate to change fputs() to
      puts().  Dealer's choice, I guess :)

 lib/table.c | 1 +
 1 file changed, 1 insertion(+)

On Tue, Aug 07, 2018 at 06:23:12PM -0400, Aaron Conole 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>
> ---
> NOTE: I chose to keep the fputs and insert an additional fputs.
>       It might alternately be appropriate to change fputs() to
>       puts().  Dealer's choice, I guess :)

It looks to me like this is a problem specifically in the formatting for
JSON tables.  If so, then the fix is more like this:

diff --git a/lib/table.c b/lib/table.c
index cd811caf5b88..19bf89262cfc 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);
 }
 

Can you check?

Ben Pfaff <blp@ovn.org> writes:

> On Tue, Aug 07, 2018 at 06:23:12PM -0400, Aaron Conole 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>
>> ---
>> NOTE: I chose to keep the fputs and insert an additional fputs.
>>       It might alternately be appropriate to change fputs() to
>>       puts().  Dealer's choice, I guess :)
>
> It looks to me like this is a problem specifically in the formatting for
> JSON tables.  If so, then the fix is more like this:
>
> diff --git a/lib/table.c b/lib/table.c
> index cd811caf5b88..19bf89262cfc 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);
>  }
>  
>
> Can you check?

Whoops!  Looks like you're right - I rushed to the wrong routine.

v2 incoming - thanks!