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

Ilya Maximets i.maximets at ovn.org
Mon Jun 1 11:31:56 UTC 2020


On 5/29/20 8:27 AM, Numan Siddique wrote:
> 
> 
> On Thu, May 28, 2020 at 11:37 PM Han Zhou <hzhou at ovn.org <mailto:hzhou at ovn.org>> wrote:
> 
>     On Mon, May 25, 2020 at 10:07 AM Ilya Maximets <i.maximets at ovn.org <mailto: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 <mailto: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 <mailto:hzhou at ovn.org>>
> 
> 
> Applied the patch and tested it.
> 
> Acked-by: Numan Siddique <numans at ovn.org <mailto:numans at ovn.org>>

Thanks, Han and Numan!
Applied to master and backported down to 2.5.

Best regards, Ilya Maximets.



More information about the dev mailing list