Message ID | 20180807222312.22895-1-aconole@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev] table: append newline when printing tables | expand |
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!
diff --git a/lib/table.c b/lib/table.c index cd811caf5..a572c3446 100644 --- a/lib/table.c +++ b/lib/table.c @@ -620,6 +620,7 @@ table_print(const struct table *table, const struct table_style *style) struct ds s = DS_EMPTY_INITIALIZER; table_format(table, style, &s); fputs(ds_cstr(&s), stdout); + fputs("\n", stdout); ds_destroy(&s); }
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(+)