[ovs-dev] [PATCH] ovsdb: Fix timeout type for wait operation.

[Numan Siddique]

On Thu, May 28, 2020 at 11:37 PM Han Zhou <hzhou at ovn.org> wrote:

> On Mon, May 25, 2020 at 10:07 AM Ilya Maximets <i.maximets at ovn.org> wrote:
> >
> > According to RFC 7047, 'timeout' is an integer field:
> >
> >  5.2.6.  Wait
> >    The "wait" object contains the following members:
> >       "op": "wait"                        required
> >       "timeout": <integer>                optional
> >       ...
> >
> > For some reason initial implementation treated it as a real number.
> >
> > This causes a build issue with clang that complains that LLONG_MAX
> > could not be represented as double:
> >
> >  ovsdb/execution.c:733:32: error: implicit conversion from 'long long'
> >                            to 'double' changes value from
> >                            9223372036854775807 to 9223372036854775808
> >             timeout_msec = MIN(LLONG_MAX, json_real(timeout));
> >                            ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  /usr/include/sys/limits.h:69:19: note: expanded from macro 'LLONG_MAX'
> >  #define LLONG_MAX       __LLONG_MAX     /* max for a long long */
> >                         ^~~~~~~~~~~
> >  /usr/include/x86/_limits.h:74:21: note: expanded from macro
> '__LLONG_MAX'
> >  #define __LLONG_MAX     0x7fffffffffffffffLL    /* max value for a long
> long */
> >                         ^~~~~~~~~~~~~~~~~~~~
> >  ./lib/util.h:90:21: note: expanded from macro 'MIN'
> >  #define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
> >                      ^  ~
> >
> > Fix that by changing parser to treat 'timeout' as integer.
> > Fixes clang build on FreeBSD 12.1 in CirrusCI.
> >
> > Fixes: f85f8ebbfac9 ("Initial implementation of OVSDB.")
> > Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> > ---
> >  ovsdb/execution.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ovsdb/execution.c b/ovsdb/execution.c
> > index e45f3d679..3a0dad5d0 100644
> > --- a/ovsdb/execution.c
> > +++ b/ovsdb/execution.c
> > @@ -712,7 +712,7 @@ ovsdb_execute_wait(struct ovsdb_execution *x, struct
> ovsdb_parser *parser,
> >      long long int timeout_msec = 0;
> >      size_t i;
> >
> > -    timeout = ovsdb_parser_member(parser, "timeout", OP_NUMBER |
> OP_OPTIONAL);
> > +    timeout = ovsdb_parser_member(parser, "timeout", OP_INTEGER |
> OP_OPTIONAL);
> >      where = ovsdb_parser_member(parser, "where", OP_ARRAY);
> >      columns_json = ovsdb_parser_member(parser, "columns",
> >                                         OP_ARRAY | OP_OPTIONAL);
> > @@ -730,7 +730,7 @@ ovsdb_execute_wait(struct ovsdb_execution *x, struct
> ovsdb_parser *parser,
> >      }
> >      if (!error) {
> >          if (timeout) {
> > -            timeout_msec = MIN(LLONG_MAX, json_real(timeout));
> > +            timeout_msec = json_integer(timeout);
> >              if (timeout_msec < 0) {
> >                  error = ovsdb_syntax_error(timeout, NULL,
> >                                             "timeout must be
> nonnegative");
> > --
> > 2.25.4
> >
>
> This looks good to me. (I didn't test myself)
>
> Acked-by: Han Zhou <hzhou at ovn.org>
>

Applied the patch and tested it.

Acked-by: Numan Siddique <numans at ovn.org>

Thanks
Numan


> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>