[ovs-dev,2.10,v2,1/2] table: Create method for resetting table formatting.

Table formatting has a local static integer that is intended to insert
line breaks between tables. This works exactly as intended, as long as
each call to table_format() is done as a single unit within the run of a
process.

When ovn-nbctl is run in daemon mode, it is a long-running process that
makes multiple calls to table_format() throughout its lifetime. After
the first call, this results in an unexpected newline prepended to table
output on each subsequent ovn-nbctl invocation.

The solution is to introduce a function to reset table formatting. This
way, the first time after resetting table formatting, no newline is
prepended.

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 lib/table.c               | 23 +++++++++++++++++------
 lib/table.h               |  1 +
 ovn/utilities/ovn-nbctl.c |  1 +
 tests/ovn-nbctl.at        | 13 +++++++++++++
 4 files changed, 32 insertions(+), 6 deletions(-)

Bleep bloop.  Greetings Mark Michelson, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 table: Create method for resetting table formatting.
The copy of the patch that failed is found in:
   /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot

On Tue, Feb 26, 2019 at 8:06 PM Mark Michelson <mmichels@redhat.com> wrote:

> Table formatting has a local static integer that is intended to insert
> line breaks between tables. This works exactly as intended, as long as
> each call to table_format() is done as a single unit within the run of a
> process.
>
> When ovn-nbctl is run in daemon mode, it is a long-running process that
> makes multiple calls to table_format() throughout its lifetime. After
> the first call, this results in an unexpected newline prepended to table
> output on each subsequent ovn-nbctl invocation.
>
> The solution is to introduce a function to reset table formatting. This
> way, the first time after resetting table formatting, no newline is
> prepended.
>
> Signed-off-by: Mark Michelson <mmichels@redhat.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
>

Acked-by: Numan Siddique <nusiddiq@redhat.com>



> ---
>  lib/table.c               | 23 +++++++++++++++++------
>  lib/table.h               |  1 +
>  ovn/utilities/ovn-nbctl.c |  1 +
>  tests/ovn-nbctl.at        | 13 +++++++++++++
>  4 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/lib/table.c b/lib/table.c
> index ab72668c7..48d18b651 100644
> --- a/lib/table.c
> +++ b/lib/table.c
> @@ -236,15 +236,18 @@ table_print_timestamp__(const struct table *table,
> struct ds *s)
>      }
>  }
>
> +static bool first_table = true;
> +
>  static void
>  table_print_table__(const struct table *table, const struct table_style
> *style,
>                      struct ds *s)
>  {
> -    static int n = 0;
>      int *widths;
>      size_t x, y;
>
> -    if (n++ > 0) {
> +    if (first_table) {
> +        first_table = false;
> +    } else {
>          ds_put_char(s, '\n');
>      }
>
> @@ -318,10 +321,11 @@ static void
>  table_print_list__(const struct table *table, const struct table_style
> *style,
>                     struct ds *s)
>  {
> -    static int n = 0;
>      size_t x, y;
>
> -    if (n++ > 0) {
> +    if (first_table) {
> +        first_table = false;
> +    } else {
>          ds_put_char(s, '\n');
>      }
>
> @@ -469,10 +473,11 @@ static void
>  table_print_csv__(const struct table *table, const struct table_style
> *style,
>                    struct ds *s)
>  {
> -    static int n = 0;
>      size_t x, y;
>
> -    if (n++ > 0) {
> +    if (first_table) {
> +        first_table = false;
> +    } else {
>          ds_put_char(s, '\n');
>      }
>
> @@ -614,6 +619,12 @@ table_format(const struct table *table, const struct
> table_style *style,
>      }
>  }
>
> +void
> +table_format_reset(void)
> +{
> +    first_table = true;
> +}
> +
>  /* Outputs 'table' on stdout in the specified 'style'. */
>  void
>  table_print(const struct table *table, const struct table_style *style)
> diff --git a/lib/table.h b/lib/table.h
> index 76e65bb70..33263e2a2 100644
> --- a/lib/table.h
> +++ b/lib/table.h
> @@ -133,6 +133,7 @@ void table_parse_cell_format(struct table_style *,
> const char *format);
>  void table_print(const struct table *, const struct table_style *);
>  void table_format(const struct table *, const struct table_style *,
>                    struct ds *);
> +void table_format_reset(void);
>  void table_usage(void);
>
>  #endif /* table.h */
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 09bbcf76a..b5ee0a6e1 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -5231,6 +5231,7 @@ server_cmd_run(struct unixctl_conn *conn, int argc,
> const char **argv_,
>      }
>
>      struct ds output = DS_EMPTY_INITIALIZER;
> +    table_format_reset();
>      for (struct ctl_command *c = commands; c < &commands[n_commands];
> c++) {
>          if (c->table) {
>              table_format(c->table, &table_style, &output);
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index a599b1bf7..7d627cdc0 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -1517,3 +1517,16 @@ AT_CHECK([ovn-nbctl create Port_Group name=pg0],
> [0], [ignore])
>  AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
>  "pg0"
>  ])])
> +
> +
> +OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [
> +dnl This test addresses a specific issue seen when running ovn-nbctl in
> +dnl daemon mode. All we have to do is ensure that each time we list
> database
> +dnl information, there is not an extra newline at the beginning of the
> output.
> +AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
> +AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
> +name                : "sw1"
> +])
> +AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
> +name                : "sw1"
> +])])
> --
> 2.14.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>