[ovs-dev] [PATCH v2] ofp-parse: Do not exit() upon a parse error.

Ben Pfaff blp at nicira.com
Mon Jul 8 18:36:08 UTC 2013


Thanks!  Applied to master.

On Mon, Jul 08, 2013 at 11:00:45AM -0700, Alex Wang wrote:
> Looks good to me, thanks.
> 
> 
> On Mon, Jul 8, 2013 at 10:16 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > Until now, failure to parse a flow in the ofp-parse module has caused the
> > program to abort immediately with a fatal error.  This makes it hard to
> > use these functions from any long-lived program.  This commit fixes the
> > problem.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > v1->v2: Fix many minor issues reported by Alex Wang.
> >
> >  lib/bundle.c               |   58 ++-
> >  lib/bundle.h               |    8 +-
> >  lib/dynamic-string.c       |    9 +-
> >  lib/dynamic-string.h       |    2 +-
> >  lib/learn.c                |  121 ++++---
> >  lib/learn.h                |    5 +-
> >  lib/meta-flow.c            |   19 +-
> >  lib/meta-flow.h            |    6 +-
> >  lib/multipath.c            |   47 ++-
> >  lib/multipath.h            |    6 +-
> >  lib/nx-match.c             |   69 +++-
> >  lib/nx-match.h             |   12 +-
> >  lib/ofp-parse.c            |  942
> > ++++++++++++++++++++++++++++++--------------
> >  lib/ofp-parse.h            |   36 +-
> >  tests/learn.at             |    2 +
> >  tests/ofproto-dpif.at      |    6 +-
> >  tests/test-bundle.c        |    9 +-
> >  tests/test-multipath.c     |    9 +-
> >  utilities/ovs-controller.c |    8 +-
> >  utilities/ovs-ofctl.c      |   71 +++-
> >  20 files changed, 990 insertions(+), 455 deletions(-)
> >
> > diff --git a/lib/bundle.c b/lib/bundle.c
> > index 78a297a..dcabaaa 100644
> > --- a/lib/bundle.c
> > +++ b/lib/bundle.c
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2011, 2012 Nicira, Inc.
> > +/* Copyright (c) 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -251,8 +251,11 @@ bundle_to_nxast(const struct ofpact_bundle *bundle,
> > struct ofpbuf *openflow)
> >      }
> >  }
> >
> > -/* Helper for bundle_parse and bundle_parse_load. */
> > -static void
> > +/* Helper for bundle_parse and bundle_parse_load.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string.*/
> > +static char * WARN_UNUSED_RESULT
> >  bundle_parse__(const char *s, char **save_ptr,
> >                 const char *fields, const char *basis, const char
> > *algorithm,
> >                 const char *slave_type, const char *dst,
> > @@ -261,12 +264,12 @@ bundle_parse__(const char *s, char **save_ptr,
> >      struct ofpact_bundle *bundle;
> >
> >      if (!slave_delim) {
> > -        ovs_fatal(0, "%s: not enough arguments to bundle action", s);
> > +        return xasprintf("%s: not enough arguments to bundle action", s);
> >      }
> >
> >      if (strcasecmp(slave_delim, "slaves")) {
> > -        ovs_fatal(0, "%s: missing slave delimiter, expected `slaves' got
> > `%s'",
> > -                   s, slave_delim);
> > +        return xasprintf("%s: missing slave delimiter, expected `slaves' "
> > +                         "got `%s'", s, slave_delim);
> >      }
> >
> >      bundle = ofpact_put_BUNDLE(ofpacts);
> > @@ -281,7 +284,7 @@ bundle_parse__(const char *s, char **save_ptr,
> >          }
> >
> >          if (!ofputil_port_from_string(slave, &slave_port)) {
> > -            ovs_fatal(0, "%s: bad port number", slave);
> > +            return xasprintf("%s: bad port number", slave);
> >          }
> >          ofpbuf_put(ofpacts, &slave_port, sizeof slave_port);
> >
> > @@ -297,7 +300,7 @@ bundle_parse__(const char *s, char **save_ptr,
> >      } else if (!strcasecmp(fields, "symmetric_l4")) {
> >          bundle->fields = NX_HASH_FIELDS_SYMMETRIC_L4;
> >      } else {
> > -        ovs_fatal(0, "%s: unknown fields `%s'", s, fields);
> > +        return xasprintf("%s: unknown fields `%s'", s, fields);
> >      }
> >
> >      if (!strcasecmp(algorithm, "active_backup")) {
> > @@ -305,25 +308,34 @@ bundle_parse__(const char *s, char **save_ptr,
> >      } else if (!strcasecmp(algorithm, "hrw")) {
> >          bundle->algorithm = NX_BD_ALG_HRW;
> >      } else {
> > -        ovs_fatal(0, "%s: unknown algorithm `%s'", s, algorithm);
> > +        return xasprintf("%s: unknown algorithm `%s'", s, algorithm);
> >      }
> >
> >      if (strcasecmp(slave_type, "ofport")) {
> > -        ovs_fatal(0, "%s: unknown slave_type `%s'", s, slave_type);
> > +        return xasprintf("%s: unknown slave_type `%s'", s, slave_type);
> >      }
> >
> >      if (dst) {
> > -        mf_parse_subfield(&bundle->dst, dst);
> > +        char *error = mf_parse_subfield(&bundle->dst, dst);
> > +        if (error) {
> > +            return error;
> > +        }
> >      }
> > +
> > +    return NULL;
> >  }
> >
> >  /* Converts a bundle action string contained in 's' to an
> > nx_action_bundle and
> > - * stores it in 'b'.  Sets 'b''s l2 pointer to NULL. */
> > -void
> > + * stores it in 'b'.  Sets 'b''s l2 pointer to NULL.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> >  bundle_parse(const char *s, struct ofpbuf *ofpacts)
> >  {
> >      char *fields, *basis, *algorithm, *slave_type, *slave_delim;
> >      char *tokstr, *save_ptr;
> > +    char *error;
> >
> >      save_ptr = NULL;
> >      tokstr = xstrdup(s);
> > @@ -333,18 +345,24 @@ bundle_parse(const char *s, struct ofpbuf *ofpacts)
> >      slave_type = strtok_r(NULL, ", ", &save_ptr);
> >      slave_delim = strtok_r(NULL, ": ", &save_ptr);
> >
> > -    bundle_parse__(s, &save_ptr, fields, basis, algorithm, slave_type,
> > NULL,
> > -                   slave_delim, ofpacts);
> > +    error = bundle_parse__(s, &save_ptr, fields, basis, algorithm,
> > slave_type,
> > +                           NULL, slave_delim, ofpacts);
> >      free(tokstr);
> > +
> > +    return error;
> >  }
> >
> >  /* Converts a bundle_load action string contained in 's' to an
> > nx_action_bundle
> > - * and stores it in 'b'.  Sets 'b''s l2 pointer to NULL. */
> > -void
> > + * and stores it in 'b'.  Sets 'b''s l2 pointer to NULL.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string.*/
> > +char * WARN_UNUSED_RESULT
> >  bundle_parse_load(const char *s, struct ofpbuf *ofpacts)
> >  {
> >      char *fields, *basis, *algorithm, *slave_type, *dst, *slave_delim;
> >      char *tokstr, *save_ptr;
> > +    char *error;
> >
> >      save_ptr = NULL;
> >      tokstr = xstrdup(s);
> > @@ -355,10 +373,12 @@ bundle_parse_load(const char *s, struct ofpbuf
> > *ofpacts)
> >      dst = strtok_r(NULL, ", ", &save_ptr);
> >      slave_delim = strtok_r(NULL, ": ", &save_ptr);
> >
> > -    bundle_parse__(s, &save_ptr, fields, basis, algorithm, slave_type,
> > dst,
> > -                   slave_delim, ofpacts);
> > +    error = bundle_parse__(s, &save_ptr, fields, basis, algorithm,
> > slave_type,
> > +                           dst, slave_delim, ofpacts);
> >
> >      free(tokstr);
> > +
> > +    return error;
> >  }
> >
> >  /* Appends a human-readable representation of 'nab' to 's'. */
> > diff --git a/lib/bundle.h b/lib/bundle.h
> > index 3e92374..dceb6e5 100644
> > --- a/lib/bundle.h
> > +++ b/lib/bundle.h
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2011, 2012 Nicira, Inc.
> > +/* Copyright (c) 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -21,6 +21,7 @@
> >  #include <stddef.h>
> >  #include <stdint.h>
> >
> > +#include "compiler.h"
> >  #include "ofp-errors.h"
> >  #include "openflow/nicira-ext.h"
> >  #include "openvswitch/types.h"
> > @@ -44,8 +45,9 @@ enum ofperr bundle_from_openflow(const struct
> > nx_action_bundle *,
> >  enum ofperr bundle_check(const struct ofpact_bundle *, ofp_port_t
> > max_ports,
> >                           const struct flow *);
> >  void bundle_to_nxast(const struct ofpact_bundle *, struct ofpbuf *of10);
> > -void bundle_parse(const char *, struct ofpbuf *ofpacts);
> > -void bundle_parse_load(const char *, struct ofpbuf *ofpacts);
> > +char *bundle_parse(const char *, struct ofpbuf *ofpacts)
> > WARN_UNUSED_RESULT;
> > +char *bundle_parse_load(const char *, struct ofpbuf *ofpacts)
> > +    WARN_UNUSED_RESULT;
> >  void bundle_format(const struct ofpact_bundle *, struct ds *);
> >
> >  #endif /* bundle.h */
> > diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c
> > index 3cccb5c..9b3e7ba 100644
> > --- a/lib/dynamic-string.c
> > +++ b/lib/dynamic-string.c
> > @@ -238,14 +238,21 @@ ds_get_line(struct ds *ds, FILE *file)
> >   * Deletes comments introduced by "#" and skips lines that contains only
> > white
> >   * space (after deleting comments).
> >   *
> > + * If 'line_numberp' is nonnull, increments '*line_numberp' by the number
> > of
> > + * lines read from 'file'.
> > + *
> >   * Returns 0 if successful, EOF if no non-blank line was found. */
> >  int
> > -ds_get_preprocessed_line(struct ds *ds, FILE *file)
> > +ds_get_preprocessed_line(struct ds *ds, FILE *file, int *line_numberp)
> >  {
> >      while (!ds_get_line(ds, file)) {
> >          char *line = ds_cstr(ds);
> >          char *comment;
> >
> > +        if (line_numberp) {
> > +            ++*line_numberp;
> > +        }
> > +
> >          /* Delete comments. */
> >          comment = strchr(line, '#');
> >          if (comment) {
> > diff --git a/lib/dynamic-string.h b/lib/dynamic-string.h
> > index b988e1f..c069586 100644
> > --- a/lib/dynamic-string.h
> > +++ b/lib/dynamic-string.h
> > @@ -58,7 +58,7 @@ void ds_put_printable(struct ds *, const char *, size_t);
> >  void ds_put_hex_dump(struct ds *ds, const void *buf_, size_t size,
> >                       uintptr_t ofs, bool ascii);
> >  int ds_get_line(struct ds *, FILE *);
> > -int ds_get_preprocessed_line(struct ds *, FILE *);
> > +int ds_get_preprocessed_line(struct ds *, FILE *, int *line_number);
> >  int ds_get_test_line(struct ds *, FILE *);
> >
> >  void ds_put_strftime(struct ds *, const char *template, time_t when, bool
> > utc)
> > diff --git a/lib/learn.c b/lib/learn.c
> > index 0cc562d..d0a4796 100644
> > --- a/lib/learn.c
> > +++ b/lib/learn.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -390,13 +390,16 @@ learn_mask(const struct ofpact_learn *learn, struct
> > flow_wildcards *wc)
> >      }
> >  }
> >
> > -static void
> > +/* Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  learn_parse_load_immediate(const char *s, struct ofpact_learn_spec *spec)
> >  {
> >      const char *full_s = s;
> >      const char *arrow = strstr(s, "->");
> >      struct mf_subfield dst;
> >      union mf_subvalue imm;
> > +    char *error;
> >
> >      memset(&imm, 0, sizeof imm);
> >      if (s[0] == '0' && (s[1] == 'x' || s[1] == 'X') && arrow) {
> > @@ -408,7 +411,7 @@ learn_parse_load_immediate(const char *s, struct
> > ofpact_learn_spec *spec)
> >          for (i = 0; i < n; i++) {
> >              int hexit = hexit_value(in[-i]);
> >              if (hexit < 0) {
> > -                ovs_fatal(0, "%s: bad hex digit in value", full_s);
> > +                return xasprintf("%s: bad hex digit in value", full_s);
> >              }
> >              out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit;
> >          }
> > @@ -418,19 +421,19 @@ learn_parse_load_immediate(const char *s, struct
> > ofpact_learn_spec *spec)
> >      }
> >
> >      if (strncmp(s, "->", 2)) {
> > -        ovs_fatal(0, "%s: missing `->' following value", full_s);
> > +        return xasprintf("%s: missing `->' following value", full_s);
> >      }
> >      s += 2;
> >
> > -    s = mf_parse_subfield(&dst, s);
> > -    if (*s != '\0') {
> > -        ovs_fatal(0, "%s: trailing garbage following destination",
> > full_s);
> > +    error = mf_parse_subfield(&dst, s);
> > +    if (error) {
> > +        return error;
> >      }
> >
> >      if (!bitwise_is_all_zeros(&imm, sizeof imm, dst.n_bits,
> >                                (8 * sizeof imm) - dst.n_bits)) {
> > -        ovs_fatal(0, "%s: value does not fit into %u bits",
> > -                  full_s, dst.n_bits);
> > +        return xasprintf("%s: value does not fit into %u bits",
> > +                         full_s, dst.n_bits);
> >      }
> >
> >      spec->n_bits = dst.n_bits;
> > @@ -438,9 +441,12 @@ learn_parse_load_immediate(const char *s, struct
> > ofpact_learn_spec *spec)
> >      spec->src_imm = imm;
> >      spec->dst_type = NX_LEARN_DST_LOAD;
> >      spec->dst = dst;
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  learn_parse_spec(const char *orig, char *name, char *value,
> >                   struct ofpact_learn_spec *spec)
> >  {
> > @@ -451,7 +457,7 @@ learn_parse_spec(const char *orig, char *name, char
> > *value,
> >
> >          error = mf_parse_value(dst, value, &imm);
> >          if (error) {
> > -            ovs_fatal(0, "%s", error);
> > +            return error;
> >          }
> >
> >          spec->n_bits = dst->n_bits;
> > @@ -465,21 +471,23 @@ learn_parse_spec(const char *orig, char *name, char
> > *value,
> >          spec->dst.n_bits = dst->n_bits;
> >      } else if (strchr(name, '[')) {
> >          /* Parse destination and check prerequisites. */
> > -        if (mf_parse_subfield(&spec->dst, name)[0] != '\0') {
> > -            ovs_fatal(0, "%s: syntax error after NXM field name `%s'",
> > -                      orig, name);
> > +        char *error;
> > +
> > +        error = mf_parse_subfield(&spec->dst, name);
> > +        if (error) {
> > +            return error;
> >          }
> >
> >          /* Parse source and check prerequisites. */
> >          if (value[0] != '\0') {
> > -            if (mf_parse_subfield(&spec->src, value)[0] != '\0') {
> > -                ovs_fatal(0, "%s: syntax error after NXM field name `%s'",
> > -                          orig, value);
> > +            error = mf_parse_subfield(&spec->src, value);
> > +            if (error) {
> > +                return error;
> >              }
> >              if (spec->src.n_bits != spec->dst.n_bits) {
> > -                ovs_fatal(0, "%s: bit widths of %s (%u) and %s (%u)
> > differ",
> > -                          orig, name, spec->src.n_bits, value,
> > -                          spec->dst.n_bits);
> > +                return xasprintf("%s: bit widths of %s (%u) and %s (%u) "
> > +                                 "differ", orig, name, spec->src.n_bits,
> > value,
> > +                                 spec->dst.n_bits);
> >              }
> >          } else {
> >              spec->src = spec->dst;
> > @@ -490,11 +498,18 @@ learn_parse_spec(const char *orig, char *name, char
> > *value,
> >          spec->dst_type = NX_LEARN_DST_MATCH;
> >      } else if (!strcmp(name, "load")) {
> >          if (value[strcspn(value, "[-")] == '-') {
> > -            learn_parse_load_immediate(value, spec);
> > +            char *error = learn_parse_load_immediate(value, spec);
> > +            if (error) {
> > +                return error;
> > +            }
> >          } else {
> >              struct ofpact_reg_move move;
> > +            char *error;
> >
> > -            nxm_parse_reg_move(&move, value);
> > +            error = nxm_parse_reg_move(&move, value);
> > +            if (error) {
> > +                return error;
> > +            }
> >
> >              spec->n_bits = move.src.n_bits;
> >              spec->src_type = NX_LEARN_SRC_FIELD;
> > @@ -503,38 +518,29 @@ learn_parse_spec(const char *orig, char *name, char
> > *value,
> >              spec->dst = move.dst;
> >          }
> >      } else if (!strcmp(name, "output")) {
> > -        if (mf_parse_subfield(&spec->src, value)[0] != '\0') {
> > -            ovs_fatal(0, "%s: syntax error after NXM field name `%s'",
> > -                      orig, name);
> > +        char *error = mf_parse_subfield(&spec->src, value);
> > +        if (error) {
> > +            return error;
> >          }
> >
> >          spec->n_bits = spec->src.n_bits;
> >          spec->src_type = NX_LEARN_SRC_FIELD;
> >          spec->dst_type = NX_LEARN_DST_OUTPUT;
> >      } else {
> > -        ovs_fatal(0, "%s: unknown keyword %s", orig, name);
> > +        return xasprintf("%s: unknown keyword %s", orig, name);
> >      }
> > +
> > +    return NULL;
> >  }
> >
> > -/* Parses 'arg' as a set of arguments to the "learn" action and appends a
> > - * matching OFPACT_LEARN action to 'ofpacts'.  ovs-ofctl(8) describes the
> > - * format parsed.
> > - *
> > - * Prints an error on stderr and aborts the program if 'arg' syntax is
> > invalid.
> > - *
> > - * If 'flow' is nonnull, then it should be the flow from a struct match
> > that is
> > - * the matching rule for the learning action.  This helps to better
> > validate
> > - * the action's arguments.
> > - *
> > - * Modifies 'arg'. */
> > -void
> > -learn_parse(char *arg, struct ofpbuf *ofpacts)
> > +/* Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +learn_parse__(char *orig, char *arg, struct ofpbuf *ofpacts)
> >  {
> > -    char *orig = xstrdup(arg);
> > -    char *name, *value;
> > -
> >      struct ofpact_learn *learn;
> >      struct match match;
> > +    char *name, *value;
> >
> >      learn = ofpact_put_LEARN(ofpacts);
> >      learn->idle_timeout = OFP_FLOW_PERMANENT;
> > @@ -547,8 +553,8 @@ learn_parse(char *arg, struct ofpbuf *ofpacts)
> >          if (!strcmp(name, "table")) {
> >              learn->table_id = atoi(value);
> >              if (learn->table_id == 255) {
> > -                ovs_fatal(0, "%s: table id 255 not valid for `learn'
> > action",
> > -                          orig);
> > +                return xasprintf("%s: table id 255 not valid for `learn' "
> > +                                 "action", orig);
> >              }
> >          } else if (!strcmp(name, "priority")) {
> >              learn->priority = atoi(value);
> > @@ -564,12 +570,16 @@ learn_parse(char *arg, struct ofpbuf *ofpacts)
> >              learn->cookie = strtoull(value, NULL, 0);
> >          } else {
> >              struct ofpact_learn_spec *spec;
> > +            char *error;
> >
> >              spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
> >              learn = ofpacts->l2;
> >              learn->n_specs++;
> >
> > -            learn_parse_spec(orig, name, value, spec);
> > +            error = learn_parse_spec(orig, name, value, spec);
> > +            if (error) {
> > +                return error;
> > +            }
> >
> >              /* Update 'match' to allow for satisfying destination
> >               * prerequisites. */
> > @@ -581,7 +591,28 @@ learn_parse(char *arg, struct ofpbuf *ofpacts)
> >      }
> >      ofpact_update_len(ofpacts, &learn->ofpact);
> >
> > +    return NULL;
> > +}
> > +
> > +/* Parses 'arg' as a set of arguments to the "learn" action and appends a
> > + * matching OFPACT_LEARN action to 'ofpacts'.  ovs-ofctl(8) describes the
> > + * format parsed.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string.
> > + *
> > + * If 'flow' is nonnull, then it should be the flow from a struct match
> > that is
> > + * the matching rule for the learning action.  This helps to better
> > validate
> > + * the action's arguments.
> > + *
> > + * Modifies 'arg'. */
> > +char * WARN_UNUSED_RESULT
> > +learn_parse(char *arg, struct ofpbuf *ofpacts)
> > +{
> > +    char *orig = xstrdup(arg);
> > +    char *error = learn_parse__(orig, arg, ofpacts);
> >      free(orig);
> > +    return error;
> >  }
> >
> >  /* Appends a description of 'learn' to 's', in the format that
> > ovs-ofctl(8)
> > diff --git a/lib/learn.h b/lib/learn.h
> > index 5bceb6e..0e676fe 100644
> > --- a/lib/learn.h
> > +++ b/lib/learn.h
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -17,6 +17,7 @@
> >  #ifndef LEARN_H
> >  #define LEARN_H 1
> >
> > +#include "compiler.h"
> >  #include "ofp-errors.h"
> >
> >  struct ds;
> > @@ -41,7 +42,7 @@ void learn_execute(const struct ofpact_learn *, const
> > struct flow *,
> >                     struct ofputil_flow_mod *, struct ofpbuf *ofpacts);
> >  void learn_mask(const struct ofpact_learn *, struct flow_wildcards *);
> >
> > -void learn_parse(char *, struct ofpbuf *ofpacts);
> > +char *learn_parse(char *, struct ofpbuf *ofpacts) WARN_UNUSED_RESULT;
> >  void learn_format(const struct ofpact_learn *, struct ds *);
> >
> >  #endif /* learn.h */
> > diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> > index 0677202..6b3b5eb 100644
> > --- a/lib/meta-flow.c
> > +++ b/lib/meta-flow.c
> > @@ -2641,7 +2641,7 @@ mf_parse_subfield_name(const char *name, int
> > name_len, bool *wild)
> >   * bit indexes.  "..end" may be omitted to indicate a single bit.
> >  "start..end"
> >   * may both be omitted (the [] are still required) to indicate an entire
> >   * field. */
> > -char *
> > +char * WARN_UNUSED_RESULT
> >  mf_parse_subfield__(struct mf_subfield *sf, const char **sp)
> >  {
> >      const struct mf_field *field;
> > @@ -2696,24 +2696,23 @@ mf_parse_subfield__(struct mf_subfield *sf, const
> > char **sp)
> >      return NULL;
> >  }
> >
> > -/* Parses a subfield from the beginning of 's' into 'sf'.  Returns the
> > first
> > - * byte in 's' following the parsed string.
> > - *
> > - * Exits with an error message if 's' has incorrect syntax.
> > +/* Parses a subfield from the entirety of 's' into 'sf'.  Returns NULL if
> > + * successful, otherwise a malloc()'d string describing the error.  The
> > caller
> > + * is responsible for freeing the returned string.
> >   *
> >   * The syntax parsed from 's' takes the form "header[start..end]" where
> >   * 'header' is the name of an NXM field and 'start' and 'end' are
> > (inclusive)
> >   * bit indexes.  "..end" may be omitted to indicate a single bit.
> >  "start..end"
> >   * may both be omitted (the [] are still required) to indicate an entire
> >   * field.  */
> > -const char *
> > +char * WARN_UNUSED_RESULT
> >  mf_parse_subfield(struct mf_subfield *sf, const char *s)
> >  {
> > -    char *msg = mf_parse_subfield__(sf, &s);
> > -    if (msg) {
> > -        ovs_fatal(0, "%s", msg);
> > +    char *error = mf_parse_subfield__(sf, &s);
> > +    if (!error && s[0]) {
> > +        error = xstrdup("unexpected input following field syntax");
> >      }
> > -    return s;
> > +    return error;
> >  }
> >
> >  void
> > diff --git a/lib/meta-flow.h b/lib/meta-flow.h
> > index a85a193..bc402dc 100644
> > --- a/lib/meta-flow.h
> > +++ b/lib/meta-flow.h
> > @@ -360,8 +360,10 @@ uint64_t mf_get_subfield(const struct mf_subfield *,
> > const struct flow *);
> >
> >
> >  void mf_format_subfield(const struct mf_subfield *, struct ds *);
> > -char *mf_parse_subfield__(struct mf_subfield *sf, const char **s);
> > -const char *mf_parse_subfield(struct mf_subfield *, const char *);
> > +char *mf_parse_subfield__(struct mf_subfield *sf, const char **s)
> > +    WARN_UNUSED_RESULT;
> > +char *mf_parse_subfield(struct mf_subfield *, const char *s)
> > +    WARN_UNUSED_RESULT;
> >
> >  enum ofperr mf_check_src(const struct mf_subfield *, const struct flow *);
> >  enum ofperr mf_check_dst(const struct mf_subfield *, const struct flow *);
> > diff --git a/lib/multipath.c b/lib/multipath.c
> > index 1be6964..6c0560d 100644
> > --- a/lib/multipath.c
> > +++ b/lib/multipath.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -195,14 +195,14 @@ multipath_algorithm(uint32_t hash, enum
> > nx_mp_algorithm algorithm,
> >  /* Parses 's_' as a set of arguments to the "multipath" action and
> > initializes
> >   * 'mp' accordingly.  ovs-ofctl(8) describes the format parsed.
> >   *
> > - * Prints an error on stderr and aborts the program if 's_' syntax is
> > - * invalid. */
> > -void
> > -multipath_parse(struct ofpact_multipath *mp, const char *s_)
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string.*/
> > +static char * WARN_UNUSED_RESULT
> > +multipath_parse__(struct ofpact_multipath *mp, const char *s_, char *s)
> >  {
> > -    char *s = xstrdup(s_);
> >      char *save_ptr = NULL;
> >      char *fields, *basis, *algorithm, *n_links_str, *arg, *dst;
> > +    char *error;
> >      int n_links;
> >
> >      fields = strtok_r(s, ", ", &save_ptr);
> > @@ -212,7 +212,7 @@ multipath_parse(struct ofpact_multipath *mp, const
> > char *s_)
> >      arg = strtok_r(NULL, ", ", &save_ptr);
> >      dst = strtok_r(NULL, ", ", &save_ptr);
> >      if (!dst) {
> > -        ovs_fatal(0, "%s: not enough arguments to multipath action", s_);
> > +        return xasprintf("%s: not enough arguments to multipath action",
> > s_);
> >      }
> >
> >      ofpact_init_MULTIPATH(mp);
> > @@ -221,7 +221,7 @@ multipath_parse(struct ofpact_multipath *mp, const
> > char *s_)
> >      } else if (!strcasecmp(fields, "symmetric_l4")) {
> >          mp->fields = NX_HASH_FIELDS_SYMMETRIC_L4;
> >      } else {
> > -        ovs_fatal(0, "%s: unknown fields `%s'", s_, fields);
> > +        return xasprintf("%s: unknown fields `%s'", s_, fields);
> >      }
> >      mp->basis = atoi(basis);
> >      if (!strcasecmp(algorithm, "modulo_n")) {
> > @@ -233,24 +233,41 @@ multipath_parse(struct ofpact_multipath *mp, const
> > char *s_)
> >      } else if (!strcasecmp(algorithm, "iter_hash")) {
> >          mp->algorithm = NX_MP_ALG_ITER_HASH;
> >      } else {
> > -        ovs_fatal(0, "%s: unknown algorithm `%s'", s_, algorithm);
> > +        return xasprintf("%s: unknown algorithm `%s'", s_, algorithm);
> >      }
> >      n_links = atoi(n_links_str);
> >      if (n_links < 1 || n_links > 65536) {
> > -        ovs_fatal(0, "%s: n_links %d is not in valid range 1 to 65536",
> > -                  s_, n_links);
> > +        return xasprintf("%s: n_links %d is not in valid range 1 to
> > 65536",
> > +                         s_, n_links);
> >      }
> >      mp->max_link = n_links - 1;
> >      mp->arg = atoi(arg);
> >
> > -    mf_parse_subfield(&mp->dst, dst);
> > +    error = mf_parse_subfield(&mp->dst, dst);
> > +    if (error) {
> > +        return error;
> > +    }
> >      if (mp->dst.n_bits < 16 && n_links > (1u << mp->dst.n_bits)) {
> > -        ovs_fatal(0, "%s: %d-bit destination field has %u possible
> > values, "
> > -                  "less than specified n_links %d",
> > -                  s_, mp->dst.n_bits, 1u << mp->dst.n_bits, n_links);
> > +        return xasprintf("%s: %d-bit destination field has %u possible "
> > +                         "values, less than specified n_links %d",
> > +                         s_, mp->dst.n_bits, 1u << mp->dst.n_bits,
> > n_links);
> >      }
> >
> > +    return NULL;
> > +}
> > +
> > +/* Parses 's_' as a set of arguments to the "multipath" action and
> > initializes
> > + * 'mp' accordingly.  ovs-ofctl(8) describes the format parsed.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> > +multipath_parse(struct ofpact_multipath *mp, const char *s_)
> > +{
> > +    char *s = xstrdup(s_);
> > +    char *error = multipath_parse__(mp, s_, s);
> >      free(s);
> > +    return error;
> >  }
> >
> >  /* Appends a description of 'mp' to 's', in the format that ovs-ofctl(8)
> > diff --git a/lib/multipath.h b/lib/multipath.h
> > index 97b5161..0673050 100644
> > --- a/lib/multipath.h
> > +++ b/lib/multipath.h
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -18,6 +18,7 @@
> >  #define MULTIPATH_H 1
> >
> >  #include <stdint.h>
> > +#include "compiler.h"
> >  #include "ofp-errors.h"
> >
> >  struct ds;
> > @@ -42,7 +43,8 @@ void multipath_to_nxast(const struct ofpact_multipath *,
> >  void multipath_execute(const struct ofpact_multipath *, struct flow *,
> >                         struct flow_wildcards *);
> >
> > -void multipath_parse(struct ofpact_multipath *, const char *);
> > +char *multipath_parse(struct ofpact_multipath *, const char *)
> > +    WARN_UNUSED_RESULT;
> >  void multipath_format(const struct ofpact_multipath *, struct ds *);
> >
> >  #endif /* multipath.h */
> > diff --git a/lib/nx-match.c b/lib/nx-match.c
> > index 8bdd8ec..3a6d7cc 100644
> > --- a/lib/nx-match.c
> > +++ b/lib/nx-match.c
> > @@ -1004,50 +1004,67 @@ oxm_match_from_string(const char *s, struct ofpbuf
> > *b)
> >      return match_len;
> >  }
> >
> > -void
> > +/* Parses 's' as a "move" action, in the form described in ovs-ofctl(8),
> > into
> > + * '*move'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> >  nxm_parse_reg_move(struct ofpact_reg_move *move, const char *s)
> >  {
> >      const char *full_s = s;
> > +    char *error;
> >
> > -    s = mf_parse_subfield(&move->src, s);
> > +    error = mf_parse_subfield__(&move->src, &s);
> > +    if (error) {
> > +        return error;
> > +    }
> >      if (strncmp(s, "->", 2)) {
> > -        ovs_fatal(0, "%s: missing `->' following source", full_s);
> > +        return xasprintf("%s: missing `->' following source", full_s);
> >      }
> >      s += 2;
> > -    s = mf_parse_subfield(&move->dst, s);
> > -    if (*s != '\0') {
> > -        ovs_fatal(0, "%s: trailing garbage following destination",
> > full_s);
> > +    error = mf_parse_subfield(&move->dst, s);
> > +    if (error) {
> > +        return error;
> >      }
> >
> >      if (move->src.n_bits != move->dst.n_bits) {
> > -        ovs_fatal(0, "%s: source field is %d bits wide but destination is
> > "
> > -                  "%d bits wide", full_s,
> > -                  move->src.n_bits, move->dst.n_bits);
> > +        return xasprintf("%s: source field is %d bits wide but
> > destination is "
> > +                         "%d bits wide", full_s,
> > +                         move->src.n_bits, move->dst.n_bits);
> >      }
> > +    return NULL;
> >  }
> >
> > -void
> > +/* Parses 's' as a "load" action, in the form described in ovs-ofctl(8),
> > into
> > + * '*load'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> >  nxm_parse_reg_load(struct ofpact_reg_load *load, const char *s)
> >  {
> >      const char *full_s = s;
> >      uint64_t value = strtoull(s, (char **) &s, 0);
> > +    char *error;
> >
> >      if (strncmp(s, "->", 2)) {
> > -        ovs_fatal(0, "%s: missing `->' following value", full_s);
> > +        return xasprintf("%s: missing `->' following value", full_s);
> >      }
> >      s += 2;
> > -    s = mf_parse_subfield(&load->dst, s);
> > -    if (*s != '\0') {
> > -        ovs_fatal(0, "%s: trailing garbage following destination",
> > full_s);
> > +    error = mf_parse_subfield(&load->dst, s);
> > +    if (error) {
> > +        return error;
> >      }
> >
> >      if (load->dst.n_bits < 64 && (value >> load->dst.n_bits) != 0) {
> > -        ovs_fatal(0, "%s: value %"PRIu64" does not fit into %d bits",
> > -                  full_s, value, load->dst.n_bits);
> > +        return xasprintf("%s: value %"PRIu64" does not fit into %d bits",
> > +                         full_s, value, load->dst.n_bits);
> >      }
> >
> >      load->subvalue.be64[0] = htonll(0);
> >      load->subvalue.be64[1] = htonll(value);
> > +    return NULL;
> >  }
> >
> >  /* nxm_format_reg_move(), nxm_format_reg_load(). */
> > @@ -1316,13 +1333,27 @@ nxm_reg_load(const struct mf_subfield *dst,
> > uint64_t src_data,
> >  }
> >
> >  /* nxm_parse_stack_action, works for both push() and pop(). */
> > -void
> > +
> > +/* Parses 's' as a "push" or "pop" action, in the form described in
> > + * ovs-ofctl(8), into '*stack_action'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> >  nxm_parse_stack_action(struct ofpact_stack *stack_action, const char *s)
> >  {
> > -    s = mf_parse_subfield(&stack_action->subfield, s);
> > +    char *error;
> > +
> > +    error = mf_parse_subfield__(&stack_action->subfield, &s);
> > +    if (error) {
> > +        return error;
> > +    }
> > +
> >      if (*s != '\0') {
> > -        ovs_fatal(0, "%s: trailing garbage following push or pop", s);
> > +        return xasprintf("%s: trailing garbage following push or pop", s);
> >      }
> > +
> > +    return NULL;
> >  }
> >
> >  void
> > diff --git a/lib/nx-match.h b/lib/nx-match.h
> > index bb63b90..b03688b 100644
> > --- a/lib/nx-match.h
> > +++ b/lib/nx-match.h
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -20,6 +20,7 @@
> >  #include <stdint.h>
> >  #include <sys/types.h>
> >  #include <netinet/in.h>
> > +#include "compiler.h"
> >  #include "flow.h"
> >  #include "ofp-errors.h"
> >  #include "openvswitch/types.h"
> > @@ -57,8 +58,10 @@ char *oxm_match_to_string(const uint8_t *, unsigned int
> > match_len);
> >  int nx_match_from_string(const char *, struct ofpbuf *);
> >  int oxm_match_from_string(const char *, struct ofpbuf *);
> >
> > -void nxm_parse_reg_move(struct ofpact_reg_move *, const char *);
> > -void nxm_parse_reg_load(struct ofpact_reg_load *, const char *);
> > +char *nxm_parse_reg_move(struct ofpact_reg_move *, const char *)
> > +    WARN_UNUSED_RESULT;
> > +char *nxm_parse_reg_load(struct ofpact_reg_load *, const char *)
> > +    WARN_UNUSED_RESULT;
> >
> >  void nxm_format_reg_move(const struct ofpact_reg_move *, struct ds *);
> >  void nxm_format_reg_load(const struct ofpact_reg_load *, struct ds *);
> > @@ -86,7 +89,8 @@ void nxm_execute_reg_load(const struct ofpact_reg_load
> > *, struct flow *);
> >  void nxm_reg_load(const struct mf_subfield *, uint64_t src_data,
> >                    struct flow *);
> >
> > -void nxm_parse_stack_action(struct ofpact_stack *, const char *);
> > +char *nxm_parse_stack_action(struct ofpact_stack *, const char *)
> > +    WARN_UNUSED_RESULT;
> >
> >  void nxm_format_stack_push(const struct ofpact_stack *, struct ds *);
> >  void nxm_format_stack_pop(const struct ofpact_stack *, struct ds *);
> > diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> > index b1e369c..609166c 100644
> > --- a/lib/ofp-parse.c
> > +++ b/lib/ofp-parse.c
> > @@ -41,88 +41,142 @@
> >
> >  VLOG_DEFINE_THIS_MODULE(ofp_parse);
> >
> > -static void ofp_fatal(const char *flow, bool verbose, const char *format,
> > ...)
> > -    NO_RETURN;
> > -
> > -static uint8_t
> > -str_to_u8(const char *str, const char *name)
> > +/* Parses 'str' as an 8-bit unsigned integer into '*valuep'.
> > + *
> > + * 'name' describes the value parsed in an error message, if any.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +str_to_u8(const char *str, const char *name, uint8_t *valuep)
> >  {
> >      int value;
> >
> > -    if (!str_to_int(str, 10, &value) || value < 0 || value > 255) {
> > -        ovs_fatal(0, "invalid %s \"%s\"", name, str);
> > +    if (!str_to_int(str, 0, &value) || value < 0 || value > 255) {
> > +        return xasprintf("invalid %s \"%s\"", name, str);
> >      }
> > -    return value;
> > +    *valuep = value;
> > +    return NULL;
> >  }
> >
> > -static uint16_t
> > -str_to_u16(const char *str, const char *name)
> > +/* Parses 'str' as a 16-bit unsigned integer into '*valuep'.
> > + *
> > + * 'name' describes the value parsed in an error message, if any.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +str_to_u16(const char *str, const char *name, uint16_t *valuep)
> >  {
> >      int value;
> >
> >      if (!str_to_int(str, 0, &value) || value < 0 || value > 65535) {
> > -        ovs_fatal(0, "invalid %s \"%s\"", name, str);
> > +        return xasprintf("invalid %s \"%s\"", name, str);
> >      }
> > -    return value;
> > +    *valuep = value;
> > +    return NULL;
> >  }
> >
> > -static uint32_t
> > -str_to_u32(const char *str)
> > +/* Parses 'str' as a 32-bit unsigned integer into '*valuep'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +str_to_u32(const char *str, uint32_t *valuep)
> >  {
> >      char *tail;
> >      uint32_t value;
> >
> >      if (!str[0]) {
> > -        ovs_fatal(0, "missing required numeric argument");
> > +        return xstrdup("missing required numeric argument");
> >      }
> >
> >      errno = 0;
> >      value = strtoul(str, &tail, 0);
> >      if (errno == EINVAL || errno == ERANGE || *tail) {
> > -        ovs_fatal(0, "invalid numeric format %s", str);
> > +        return xasprintf("invalid numeric format %s", str);
> >      }
> > -    return value;
> > +    *valuep = value;
> > +    return NULL;
> >  }
> >
> > -static uint64_t
> > -str_to_u64(const char *str)
> > +/* Parses 'str' as an 64-bit unsigned integer into '*valuep'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +str_to_u64(const char *str, uint64_t *valuep)
> >  {
> >      char *tail;
> >      uint64_t value;
> >
> >      if (!str[0]) {
> > -        ovs_fatal(0, "missing required numeric argument");
> > +        return xstrdup("missing required numeric argument");
> >      }
> >
> >      errno = 0;
> >      value = strtoull(str, &tail, 0);
> >      if (errno == EINVAL || errno == ERANGE || *tail) {
> > -        ovs_fatal(0, "invalid numeric format %s", str);
> > +        return xasprintf("invalid numeric format %s", str);
> >      }
> > -    return value;
> > +    *valuep = value;
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'str' as an 64-bit unsigned integer in network byte order into
> > + * '*valuep'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +str_to_be64(const char *str, ovs_be64 *valuep)
> > +{
> > +    uint64_t value;
> > +    char *error;
> > +
> > +    error = str_to_u64(str, &value);
> > +    if (!error) {
> > +        *valuep = htonll(value);
> > +    }
> > +    return error;
> > +}
> > +
> > +/* Parses 'str' as an Ethernet address into 'mac'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  str_to_mac(const char *str, uint8_t mac[6])
> >  {
> >      if (sscanf(str, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(mac))
> >          != ETH_ADDR_SCAN_COUNT) {
> > -        ovs_fatal(0, "invalid mac address %s", str);
> > +        return xasprintf("invalid mac address %s", str);
> >      }
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'str' as an IP address into '*ip'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  str_to_ip(const char *str, ovs_be32 *ip)
> >  {
> >      struct in_addr in_addr;
> >
> >      if (lookup_ip(str, &in_addr)) {
> > -        ovs_fatal(0, "%s: could not convert to IP address", str);
> > +        return xasprintf("%s: could not convert to IP address", str);
> >      }
> >      *ip = in_addr.s_addr;
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to an "enqueue" action, and appends such
> > an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_enqueue(char *arg, struct ofpbuf *ofpacts)
> >  {
> >      char *sp = NULL;
> > @@ -131,33 +185,48 @@ parse_enqueue(char *arg, struct ofpbuf *ofpacts)
> >      struct ofpact_enqueue *enqueue;
> >
> >      if (port == NULL || queue == NULL) {
> > -        ovs_fatal(0, "\"enqueue\" syntax is \"enqueue:PORT:QUEUE\"");
> > +        return xstrdup("\"enqueue\" syntax is \"enqueue:PORT:QUEUE\"");
> >      }
> >
> >      enqueue = ofpact_put_ENQUEUE(ofpacts);
> > -    enqueue->port = u16_to_ofp(str_to_u32(port));
> > -    enqueue->queue = str_to_u32(queue);
> > +    if (!ofputil_port_from_string(port, &enqueue->port)) {
> > +        return xasprintf("%s: enqueue to unknown port", port);
> > +    }
> > +    return str_to_u32(queue, &enqueue->queue);
> >  }
> >
> > -static void
> > -parse_output(char *arg, struct ofpbuf *ofpacts)
> > +/* Parses 'arg' as the argument to an "output" action, and appends such an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +parse_output(const char *arg, struct ofpbuf *ofpacts)
> >  {
> >      if (strchr(arg, '[')) {
> >          struct ofpact_output_reg *output_reg;
> >
> >          output_reg = ofpact_put_OUTPUT_REG(ofpacts);
> > -        mf_parse_subfield(&output_reg->src, arg);
> >          output_reg->max_len = UINT16_MAX;
> > +        return mf_parse_subfield(&output_reg->src, arg);
> >      } else {
> >          struct ofpact_output *output;
> >
> >          output = ofpact_put_OUTPUT(ofpacts);
> > -        output->port = u16_to_ofp(str_to_u32(arg));
> >          output->max_len = output->port == OFPP_CONTROLLER ? UINT16_MAX :
> > 0;
> > +        if (!ofputil_port_from_string(arg, &output->port)) {
> > +            return xasprintf("%s: output to unknown port", arg);
> > +        }
> > +        return NULL;
> >      }
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to an "resubmit" action, and appends such
> > an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_resubmit(char *arg, struct ofpbuf *ofpacts)
> >  {
> >      struct ofpact_resubmit *resubmit;
> > @@ -168,22 +237,39 @@ parse_resubmit(char *arg, struct ofpbuf *ofpacts)
> >      in_port_s = strsep(&arg, ",");
> >      if (in_port_s && in_port_s[0]) {
> >          if (!ofputil_port_from_string(in_port_s, &resubmit->in_port)) {
> > -            ovs_fatal(0, "%s: resubmit to unknown port", in_port_s);
> > +            return xasprintf("%s: resubmit to unknown port", in_port_s);
> >          }
> >      } else {
> >          resubmit->in_port = OFPP_IN_PORT;
> >      }
> >
> >      table_s = strsep(&arg, ",");
> > -    resubmit->table_id = table_s && table_s[0] ? str_to_u32(table_s) :
> > 255;
> > +    if (table_s && table_s[0]) {
> > +        uint32_t table_id;
> > +        char *error;
> > +
> > +        error = str_to_u32(table_s, &table_id);
> > +        if (error) {
> > +            return error;
> > +        }
> > +        resubmit->table_id = table_id;
> > +    } else {
> > +        resubmit->table_id = 255;
> > +    }
> >
> >      if (resubmit->in_port == OFPP_IN_PORT && resubmit->table_id == 255) {
> > -        ovs_fatal(0, "at least one \"in_port\" or \"table\" must be
> > specified "
> > -                  " on resubmit");
> > +        return xstrdup("at least one \"in_port\" or \"table\" must be "
> > +                       "specified  on resubmit");
> >      }
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to a "note" action, and appends such an
> > action
> > + * to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_note(const char *arg, struct ofpbuf *ofpacts)
> >  {
> >      struct ofpact_note *note;
> > @@ -202,7 +288,7 @@ parse_note(const char *arg, struct ofpbuf *ofpacts)
> >
> >          byte = hexits_value(arg, 2, &ok);
> >          if (!ok) {
> > -            ovs_fatal(0, "bad hex digit in `note' argument");
> > +            return xstrdup("bad hex digit in `note' argument");
> >          }
> >          ofpbuf_put(ofpacts, &byte, 1);
> >
> > @@ -212,26 +298,45 @@ parse_note(const char *arg, struct ofpbuf *ofpacts)
> >          arg += 2;
> >      }
> >      ofpact_update_len(ofpacts, &note->ofpact);
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to a "fin_timeout" action, and appends
> > such an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_fin_timeout(struct ofpbuf *b, char *arg)
> >  {
> >      struct ofpact_fin_timeout *oft = ofpact_put_FIN_TIMEOUT(b);
> >      char *key, *value;
> >
> >      while (ofputil_parse_key_value(&arg, &key, &value)) {
> > +        char *error;
> > +
> >          if (!strcmp(key, "idle_timeout")) {
> > -            oft->fin_idle_timeout = str_to_u16(value, key);
> > +            error =  str_to_u16(value, key, &oft->fin_idle_timeout);
> >          } else if (!strcmp(key, "hard_timeout")) {
> > -            oft->fin_hard_timeout = str_to_u16(value, key);
> > +            error = str_to_u16(value, key, &oft->fin_hard_timeout);
> >          } else {
> > -            ovs_fatal(0, "invalid key '%s' in 'fin_timeout' argument",
> > key);
> > +            error = xasprintf("invalid key '%s' in 'fin_timeout'
> > argument",
> > +                              key);
> > +        }
> > +
> > +        if (error) {
> > +            return error;
> >          }
> >      }
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to a "controller" action, and appends
> > such an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_controller(struct ofpbuf *b, char *arg)
> >  {
> >      enum ofp_packet_in_reason reason = OFPR_ACTION;
> > @@ -241,22 +346,31 @@ parse_controller(struct ofpbuf *b, char *arg)
> >      if (!arg[0]) {
> >          /* Use defaults. */
> >      } else if (strspn(arg, "0123456789") == strlen(arg)) {
> > -        max_len = str_to_u16(arg, "max_len");
> > +        char *error = str_to_u16(arg, "max_len", &max_len);
> > +        if (error) {
> > +            return error;
> > +        }
> >      } else {
> >          char *name, *value;
> >
> >          while (ofputil_parse_key_value(&arg, &name, &value)) {
> >              if (!strcmp(name, "reason")) {
> >                  if (!ofputil_packet_in_reason_from_string(value,
> > &reason)) {
> > -                    ovs_fatal(0, "unknown reason \"%s\"", value);
> > +                    return xasprintf("unknown reason \"%s\"", value);
> >                  }
> >              } else if (!strcmp(name, "max_len")) {
> > -                max_len = str_to_u16(value, "max_len");
> > +                char *error = str_to_u16(value, "max_len", &max_len);
> > +                if (error) {
> > +                    return error;
> > +                }
> >              } else if (!strcmp(name, "id")) {
> > -                controller_id = str_to_u16(value, "id");
> > +                char *error = str_to_u16(value, "id", &controller_id);
> > +                if (error) {
> > +                    return error;
> > +                }
> >              } else {
> > -                ovs_fatal(0, "unknown key \"%s\" parsing controller
> > action",
> > -                          name);
> > +                return xasprintf("unknown key \"%s\" parsing controller "
> > +                                 "action", name);
> >              }
> >          }
> >      }
> > @@ -275,6 +389,8 @@ parse_controller(struct ofpbuf *b, char *arg)
> >          controller->reason = reason;
> >          controller->controller_id = controller_id;
> >      }
> > +
> > +    return NULL;
> >  }
> >
> >  static void
> > @@ -290,7 +406,12 @@ parse_noargs_dec_ttl(struct ofpbuf *b)
> >      ofpact_update_len(b, &ids->ofpact);
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to a "dec_ttl" action, and appends such an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_dec_ttl(struct ofpbuf *b, char *arg)
> >  {
> >      if (*arg == '\0') {
> > @@ -310,68 +431,99 @@ parse_dec_ttl(struct ofpbuf *b, char *arg)
> >              ids->n_controllers++;
> >          }
> >          if (!ids->n_controllers) {
> > -            ovs_fatal(0, "dec_ttl_cnt_ids: expected at least one
> > controller "
> > -                      "id.");
> > +            return xstrdup("dec_ttl_cnt_ids: expected at least one
> > controller "
> > +                           "id.");
> >          }
> >          ofpact_update_len(b, &ids->ofpact);
> >      }
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to a "set_mpls_ttl" action, and appends
> > such an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_set_mpls_ttl(struct ofpbuf *b, const char *arg)
> >  {
> >      struct ofpact_mpls_ttl *mpls_ttl = ofpact_put_SET_MPLS_TTL(b);
> >
> >      if (*arg == '\0') {
> > -        ovs_fatal(0, "parse_set_mpls_ttl: expected ttl.");
> > +        return xstrdup("parse_set_mpls_ttl: expected ttl.");
> >      }
> >
> >      mpls_ttl->ttl = atoi(arg);
> > +    return NULL;
> >  }
> >
> > -static void
> > -set_field_parse(const char *arg, struct ofpbuf *ofpacts)
> > +/* Parses a "set_field" action with argument 'arg', appending the parsed
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +set_field_parse__(char *arg, struct ofpbuf *ofpacts)
> >  {
> > -    char *orig = xstrdup(arg);
> >      struct ofpact_reg_load *load = ofpact_put_REG_LOAD(ofpacts);
> >      char *value;
> >      char *delim;
> >      char *key;
> >      const struct mf_field *mf;
> > -    const char *error;
> > +    char *error;
> >      union mf_value mf_value;
> >
> > -    value = orig;
> > -    delim = strstr(orig, "->");
> > +    value = arg;
> > +    delim = strstr(arg, "->");
> >      if (!delim) {
> > -        ovs_fatal(0, "%s: missing `->'", orig);
> > +        return xasprintf("%s: missing `->'", arg);
> >      }
> >      if (strlen(delim) <= strlen("->")) {
> > -        ovs_fatal(0, "%s: missing field name following `->'", orig);
> > +        return xasprintf("%s: missing field name following `->'", arg);
> >      }
> >
> >      key = delim + strlen("->");
> >      mf = mf_from_name(key);
> >      if (!mf) {
> > -        ovs_fatal(0, "%s is not valid oxm field name", key);
> > +        return xasprintf("%s is not a valid OXM field name", key);
> >      }
> >      if (!mf->writable) {
> > -        ovs_fatal(0, "%s is not allowed to set", key);
> > +        return xasprintf("%s is read-only", key);
> >      }
> >
> >      delim[0] = '\0';
> >      error = mf_parse_value(mf, value, &mf_value);
> >      if (error) {
> > -        ovs_fatal(0, "%s", error);
> > +        return error;
> >      }
> >      if (!mf_is_value_valid(mf, &mf_value)) {
> > -        ovs_fatal(0, "%s is not valid valid for field %s", value, key);
> > +        return xasprintf("%s is not a valid value for field %s", value,
> > key);
> >      }
> >      ofpact_set_field_init(load, mf, &mf_value);
> > -    free(orig);
> > +
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to a "set_field" action, and appends such
> > an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> > +set_field_parse(const char *arg, struct ofpbuf *ofpacts)
> > +{
> > +    char *copy = xstrdup(arg);
> > +    char *error = set_field_parse__(copy, ofpacts);
> > +    free(copy);
> > +    return error;
> > +}
> > +
> > +/* Parses 'arg' as the argument to a "write_metadata" instruction, and
> > appends
> > + * such an action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_metadata(struct ofpbuf *b, char *arg)
> >  {
> >      struct ofpact_metadata *om;
> > @@ -380,51 +532,73 @@ parse_metadata(struct ofpbuf *b, char *arg)
> >      om = ofpact_put_WRITE_METADATA(b);
> >
> >      if (mask) {
> > +        char *error;
> > +
> >          *mask = '\0';
> > -        om->mask = htonll(str_to_u64(mask + 1));
> > +        error = str_to_be64(mask + 1, &om->mask);
> > +        if (error) {
> > +            return error;
> > +        }
> >      } else {
> >          om->mask = htonll(UINT64_MAX);
> >      }
> >
> > -    om->metadata = htonll(str_to_u64(arg));
> > +    return str_to_be64(arg, &om->metadata);
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to a "sample" action, and appends such an
> > + * action to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_sample(struct ofpbuf *b, char *arg)
> >  {
> >      struct ofpact_sample *os = ofpact_put_SAMPLE(b);
> >      char *key, *value;
> >
> >      while (ofputil_parse_key_value(&arg, &key, &value)) {
> > +        char *error = NULL;
> > +
> >          if (!strcmp(key, "probability")) {
> > -            os->probability = str_to_u16(value, "probability");
> > -            if (os->probability == 0) {
> > -                ovs_fatal(0, "invalid probability value \"%s\"", value);
> > +            error = str_to_u16(value, "probability", &os->probability);
> > +            if (!error && os->probability == 0) {
> > +                error = xasprintf("invalid probability value \"%s\"",
> > value);
> >              }
> >          } else if (!strcmp(key, "collector_set_id")) {
> > -            os->collector_set_id = str_to_u32(value);
> > +            error = str_to_u32(value, &os->collector_set_id);
> >          } else if (!strcmp(key, "obs_domain_id")) {
> > -            os->obs_domain_id = str_to_u32(value);
> > +            error = str_to_u32(value, &os->obs_domain_id);
> >          } else if (!strcmp(key, "obs_point_id")) {
> > -            os->obs_point_id = str_to_u32(value);
> > +            error = str_to_u32(value, &os->obs_point_id);
> >          } else {
> > -            ovs_fatal(0, "invalid key \"%s\" in \"sample\" argument",
> > -                      key);
> > +            error = xasprintf("invalid key \"%s\" in \"sample\" argument",
> > +                              key);
> > +        }
> > +        if (error) {
> > +            return error;
> >          }
> >      }
> >      if (os->probability == 0) {
> > -        ovs_fatal(0, "non-zero \"probability\" must be specified on
> > sample");
> > +        return xstrdup("non-zero \"probability\" must be specified on
> > sample");
> >      }
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to action 'code', and appends such an
> > action to
> > + * 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_named_action(enum ofputil_action_code code,
> >                     char *arg, struct ofpbuf *ofpacts)
> >  {
> > +    size_t orig_size = ofpacts->size;
> >      struct ofpact_tunnel *tunnel;
> > -    uint16_t vid;
> > +    char *error = NULL;
> >      uint16_t ethertype;
> > -    ovs_be32 ip;
> > +    uint16_t vid;
> >      uint8_t pcp;
> >      uint8_t tos;
> >
> > @@ -434,30 +608,37 @@ parse_named_action(enum ofputil_action_code code,
> >
> >      case OFPUTIL_OFPAT10_OUTPUT:
> >      case OFPUTIL_OFPAT11_OUTPUT:
> > -        parse_output(arg, ofpacts);
> > +        error = parse_output(arg, ofpacts);
> >          break;
> >
> >      case OFPUTIL_OFPAT10_SET_VLAN_VID:
> >      case OFPUTIL_OFPAT11_SET_VLAN_VID:
> > -        vid = str_to_u32(arg);
> > +        error = str_to_u16(arg, "VLAN VID", &vid);
> > +        if (error) {
> > +            return error;
> > +        }
> > +
> >          if (vid & ~VLAN_VID_MASK) {
> > -            ovs_fatal(0, "%s: not a valid VLAN VID", arg);
> > +            return xasprintf("%s: not a valid VLAN VID", arg);
> >          }
> >          ofpact_put_SET_VLAN_VID(ofpacts)->vlan_vid = vid;
> >          break;
> >
> >      case OFPUTIL_OFPAT10_SET_VLAN_PCP:
> >      case OFPUTIL_OFPAT11_SET_VLAN_PCP:
> > -        pcp = str_to_u32(arg);
> > +        error = str_to_u8(arg, "VLAN PCP", &pcp);
> > +        if (error) {
> > +            return error;
> > +        }
> > +
> >          if (pcp & ~7) {
> > -            ovs_fatal(0, "%s: not a valid VLAN PCP", arg);
> > +            return xasprintf("%s: not a valid VLAN PCP", arg);
> >          }
> >          ofpact_put_SET_VLAN_PCP(ofpacts)->vlan_pcp = pcp;
> >          break;
> >
> >      case OFPUTIL_OFPAT12_SET_FIELD:
> > -        set_field_parse(arg, ofpacts);
> > -        break;
> > +        return set_field_parse(arg, ofpacts);
> >
> >      case OFPUTIL_OFPAT10_STRIP_VLAN:
> >      case OFPUTIL_OFPAT11_POP_VLAN:
> > @@ -465,46 +646,52 @@ parse_named_action(enum ofputil_action_code code,
> >          break;
> >
> >      case OFPUTIL_OFPAT11_PUSH_VLAN:
> > -        ethertype = str_to_u16(arg, "ethertype");
> > +        error = str_to_u16(arg, "ethertype", &ethertype);
> > +        if (error) {
> > +            return error;
> > +        }
> > +
> >          if (ethertype != ETH_TYPE_VLAN_8021Q) {
> >              /* XXX ETH_TYPE_VLAN_8021AD case isn't supported */
> > -            ovs_fatal(0, "%s: not a valid VLAN ethertype", arg);
> > +            return xasprintf("%s: not a valid VLAN ethertype", arg);
> >          }
> > +
> >          ofpact_put_PUSH_VLAN(ofpacts);
> >          break;
> >
> >      case OFPUTIL_OFPAT11_SET_QUEUE:
> > -        ofpact_put_SET_QUEUE(ofpacts)->queue_id = str_to_u32(arg);
> > +        error = str_to_u32(arg, &ofpact_put_SET_QUEUE(ofpacts)->queue_id);
> >          break;
> >
> > -
> >      case OFPUTIL_OFPAT10_SET_DL_SRC:
> >      case OFPUTIL_OFPAT11_SET_DL_SRC:
> > -        str_to_mac(arg, ofpact_put_SET_ETH_SRC(ofpacts)->mac);
> > +        error = str_to_mac(arg, ofpact_put_SET_ETH_SRC(ofpacts)->mac);
> >          break;
> >
> >      case OFPUTIL_OFPAT10_SET_DL_DST:
> >      case OFPUTIL_OFPAT11_SET_DL_DST:
> > -        str_to_mac(arg, ofpact_put_SET_ETH_DST(ofpacts)->mac);
> > +        error = str_to_mac(arg, ofpact_put_SET_ETH_DST(ofpacts)->mac);
> >          break;
> >
> >      case OFPUTIL_OFPAT10_SET_NW_SRC:
> >      case OFPUTIL_OFPAT11_SET_NW_SRC:
> > -        str_to_ip(arg, &ip);
> > -        ofpact_put_SET_IPV4_SRC(ofpacts)->ipv4 = ip;
> > +        error = str_to_ip(arg, &ofpact_put_SET_IPV4_SRC(ofpacts)->ipv4);
> >          break;
> >
> >      case OFPUTIL_OFPAT10_SET_NW_DST:
> >      case OFPUTIL_OFPAT11_SET_NW_DST:
> > -        str_to_ip(arg, &ip);
> > -        ofpact_put_SET_IPV4_DST(ofpacts)->ipv4 = ip;
> > +        error = str_to_ip(arg, &ofpact_put_SET_IPV4_DST(ofpacts)->ipv4);
> >          break;
> >
> >      case OFPUTIL_OFPAT10_SET_NW_TOS:
> >      case OFPUTIL_OFPAT11_SET_NW_TOS:
> > -        tos = str_to_u32(arg);
> > +        error = str_to_u8(arg, "TOS", &tos);
> > +        if (error) {
> > +            return error;
> > +        }
> > +
> >          if (tos & ~IP_DSCP_MASK) {
> > -            ovs_fatal(0, "%s: not a valid TOS", arg);
> > +            return xasprintf("%s: not a valid TOS", arg);
> >          }
> >          ofpact_put_SET_IPV4_DSCP(ofpacts)->dscp = tos;
> >          break;
> > @@ -514,35 +701,37 @@ parse_named_action(enum ofputil_action_code code,
> >
> >      case OFPUTIL_OFPAT10_SET_TP_SRC:
> >      case OFPUTIL_OFPAT11_SET_TP_SRC:
> > -        ofpact_put_SET_L4_SRC_PORT(ofpacts)->port = str_to_u32(arg);
> > +        error = str_to_u16(arg, "source port",
> > +                           &ofpact_put_SET_L4_SRC_PORT(ofpacts)->port);
> >          break;
> >
> >      case OFPUTIL_OFPAT10_SET_TP_DST:
> >      case OFPUTIL_OFPAT11_SET_TP_DST:
> > -        ofpact_put_SET_L4_DST_PORT(ofpacts)->port = str_to_u32(arg);
> > +        error = str_to_u16(arg, "destination port",
> > +                           &ofpact_put_SET_L4_DST_PORT(ofpacts)->port);
> >          break;
> >
> >      case OFPUTIL_OFPAT10_ENQUEUE:
> > -        parse_enqueue(arg, ofpacts);
> > +        error = parse_enqueue(arg, ofpacts);
> >          break;
> >
> >      case OFPUTIL_NXAST_RESUBMIT:
> > -        parse_resubmit(arg, ofpacts);
> > +        error = parse_resubmit(arg, ofpacts);
> >          break;
> >
> >      case OFPUTIL_NXAST_SET_TUNNEL:
> >      case OFPUTIL_NXAST_SET_TUNNEL64:
> >          tunnel = ofpact_put_SET_TUNNEL(ofpacts);
> >          tunnel->ofpact.compat = code;
> > -        tunnel->tun_id = str_to_u64(arg);
> > +        error = str_to_u64(arg, &tunnel->tun_id);
> >          break;
> >
> >      case OFPUTIL_NXAST_WRITE_METADATA:
> > -        parse_metadata(ofpacts, arg);
> > +        error = parse_metadata(ofpacts, arg);
> >          break;
> >
> >      case OFPUTIL_NXAST_SET_QUEUE:
> > -        ofpact_put_SET_QUEUE(ofpacts)->queue_id = str_to_u32(arg);
> > +        error = str_to_u32(arg, &ofpact_put_SET_QUEUE(ofpacts)->queue_id);
> >          break;
> >
> >      case OFPUTIL_NXAST_POP_QUEUE:
> > @@ -550,27 +739,27 @@ parse_named_action(enum ofputil_action_code code,
> >          break;
> >
> >      case OFPUTIL_NXAST_REG_MOVE:
> > -        nxm_parse_reg_move(ofpact_put_REG_MOVE(ofpacts), arg);
> > +        error = nxm_parse_reg_move(ofpact_put_REG_MOVE(ofpacts), arg);
> >          break;
> >
> >      case OFPUTIL_NXAST_REG_LOAD:
> > -        nxm_parse_reg_load(ofpact_put_REG_LOAD(ofpacts), arg);
> > +        error = nxm_parse_reg_load(ofpact_put_REG_LOAD(ofpacts), arg);
> >          break;
> >
> >      case OFPUTIL_NXAST_NOTE:
> > -        parse_note(arg, ofpacts);
> > +        error = parse_note(arg, ofpacts);
> >          break;
> >
> >      case OFPUTIL_NXAST_MULTIPATH:
> > -        multipath_parse(ofpact_put_MULTIPATH(ofpacts), arg);
> > +        error = multipath_parse(ofpact_put_MULTIPATH(ofpacts), arg);
> >          break;
> >
> >      case OFPUTIL_NXAST_BUNDLE:
> > -        bundle_parse(arg, ofpacts);
> > +        error = bundle_parse(arg, ofpacts);
> >          break;
> >
> >      case OFPUTIL_NXAST_BUNDLE_LOAD:
> > -        bundle_parse_load(arg, ofpacts);
> > +        error = bundle_parse_load(arg, ofpacts);
> >          break;
> >
> >      case OFPUTIL_NXAST_RESUBMIT_TABLE:
> > @@ -579,7 +768,7 @@ parse_named_action(enum ofputil_action_code code,
> >          NOT_REACHED();
> >
> >      case OFPUTIL_NXAST_LEARN:
> > -        learn_parse(arg, ofpacts);
> > +        error = learn_parse(arg, ofpacts);
> >          break;
> >
> >      case OFPUTIL_NXAST_EXIT:
> > @@ -587,12 +776,12 @@ parse_named_action(enum ofputil_action_code code,
> >          break;
> >
> >      case OFPUTIL_NXAST_DEC_TTL:
> > -        parse_dec_ttl(ofpacts, arg);
> > +        error = parse_dec_ttl(ofpacts, arg);
> >          break;
> >
> >      case OFPUTIL_NXAST_SET_MPLS_TTL:
> >      case OFPUTIL_OFPAT11_SET_MPLS_TTL:
> > -        parse_set_mpls_ttl(ofpacts, arg);
> > +        error = parse_set_mpls_ttl(ofpacts, arg);
> >          break;
> >
> >      case OFPUTIL_OFPAT11_DEC_MPLS_TTL:
> > @@ -601,69 +790,90 @@ parse_named_action(enum ofputil_action_code code,
> >          break;
> >
> >      case OFPUTIL_NXAST_FIN_TIMEOUT:
> > -        parse_fin_timeout(ofpacts, arg);
> > +        error = parse_fin_timeout(ofpacts, arg);
> >          break;
> >
> >      case OFPUTIL_NXAST_CONTROLLER:
> > -        parse_controller(ofpacts, arg);
> > +        error = parse_controller(ofpacts, arg);
> >          break;
> >
> >      case OFPUTIL_OFPAT11_PUSH_MPLS:
> >      case OFPUTIL_NXAST_PUSH_MPLS:
> > -        ofpact_put_PUSH_MPLS(ofpacts)->ethertype =
> > -            htons(str_to_u16(arg, "push_mpls"));
> > +        error = str_to_u16(arg, "push_mpls", &ethertype);
> > +        if (!error) {
> > +            ofpact_put_PUSH_MPLS(ofpacts)->ethertype = htons(ethertype);
> > +        }
> >          break;
> >
> >      case OFPUTIL_OFPAT11_POP_MPLS:
> >      case OFPUTIL_NXAST_POP_MPLS:
> > -        ofpact_put_POP_MPLS(ofpacts)->ethertype =
> > -            htons(str_to_u16(arg, "pop_mpls"));
> > +        error = str_to_u16(arg, "pop_mpls", &ethertype);
> > +        if (!error) {
> > +            ofpact_put_POP_MPLS(ofpacts)->ethertype = htons(ethertype);
> > +        }
> >          break;
> >
> >      case OFPUTIL_NXAST_STACK_PUSH:
> > -        nxm_parse_stack_action(ofpact_put_STACK_PUSH(ofpacts), arg);
> > +        error = nxm_parse_stack_action(ofpact_put_STACK_PUSH(ofpacts),
> > arg);
> >          break;
> >      case OFPUTIL_NXAST_STACK_POP:
> > -        nxm_parse_stack_action(ofpact_put_STACK_POP(ofpacts), arg);
> > +        error = nxm_parse_stack_action(ofpact_put_STACK_POP(ofpacts),
> > arg);
> >          break;
> >
> >      case OFPUTIL_NXAST_SAMPLE:
> > -        parse_sample(ofpacts, arg);
> > +        error = parse_sample(ofpacts, arg);
> >          break;
> >      }
> > +
> > +    if (error) {
> > +        ofpacts->size = orig_size;
> > +    }
> > +    return error;
> >  }
> >
> > -static bool
> > +/* Parses action 'act', with argument 'arg', and appends a parsed version
> > to
> > + * 'ofpacts'.
> > + *
> > + * 'n_actions' specifies the number of actions already parsed (for proper
> > + * handling of "drop" actions).
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  str_to_ofpact__(char *pos, char *act, char *arg,
> >                  struct ofpbuf *ofpacts, int n_actions)
> >  {
> >      int code = ofputil_action_code_from_name(act);
> >      if (code >= 0) {
> > -        parse_named_action(code, arg, ofpacts);
> > +        return parse_named_action(code, arg, ofpacts);
> >      } else if (!strcasecmp(act, "drop")) {
> >          if (n_actions) {
> > -            ovs_fatal(0, "Drop actions must not be preceded by other "
> > -                      "actions");
> > +            return xstrdup("Drop actions must not be preceded by other "
> > +                           "actions");
> >          } else if (ofputil_parse_key_value(&pos, &act, &arg)) {
> > -            ovs_fatal(0, "Drop actions must not be followed by other "
> > -                      "actions");
> > +            return xstrdup("Drop actions must not be followed by other "
> > +                           "actions");
> >          }
> > -        return false;
> >      } else {
> >          ofp_port_t port;
> >          if (ofputil_port_from_string(act, &port)) {
> >              ofpact_put_OUTPUT(ofpacts)->port = port;
> >          } else {
> > -            ovs_fatal(0, "Unknown action: %s", act);
> > +            return xasprintf("Unknown action: %s", act);
> >          }
> >      }
> >
> > -    return true;
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'str' as a series of actions, and appends them to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  str_to_ofpacts(char *str, struct ofpbuf *ofpacts)
> >  {
> > +    size_t orig_size = ofpacts->size;
> >      char *pos, *act, *arg;
> >      enum ofperr error;
> >      int n_actions;
> > @@ -671,24 +881,34 @@ str_to_ofpacts(char *str, struct ofpbuf *ofpacts)
> >      pos = str;
> >      n_actions = 0;
> >      while (ofputil_parse_key_value(&pos, &act, &arg)) {
> > -        if (!str_to_ofpact__(pos, act, arg, ofpacts, n_actions)) {
> > -            break;
> > +        char *error = str_to_ofpact__(pos, act, arg, ofpacts, n_actions);
> > +        if (error) {
> > +            ofpacts->size = orig_size;
> > +            return error;
> >          }
> >          n_actions++;
> >      }
> >
> >      error = ofpacts_verify(ofpacts->data, ofpacts->size);
> >      if (error) {
> > -        ovs_fatal(0, "Incorrect action ordering");
> > +        ofpacts->size = orig_size;
> > +        return xstrdup("Incorrect action ordering");
> >      }
> >
> >      ofpact_pad(ofpacts);
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'arg' as the argument to instruction 'type', and appends such an
> > + * instruction to 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_named_instruction(enum ovs_instruction_type type,
> >                          char *arg, struct ofpbuf *ofpacts)
> >  {
> > +    char *error_s = NULL;
> >      enum ofperr error;
> >
> >      switch (type) {
> > @@ -698,7 +918,7 @@ parse_named_instruction(enum ovs_instruction_type type,
> >
> >      case OVSINST_OFPIT11_WRITE_ACTIONS:
> >          /* XXX */
> > -        ovs_fatal(0, "instruction write-actions is not supported yet");
> > +        error_s = xstrdup("instruction write-actions is not supported
> > yet");
> >          break;
> >
> >      case OVSINST_OFPIT11_CLEAR_ACTIONS:
> > @@ -706,35 +926,45 @@ parse_named_instruction(enum ovs_instruction_type
> > type,
> >          break;
> >
> >      case OVSINST_OFPIT13_METER:
> > -        ofpact_put_METER(ofpacts)->meter_id = str_to_u32(arg);
> > +        error_s = str_to_u32(arg, &ofpact_put_METER(ofpacts)->meter_id);
> >          break;
> >
> >      case OVSINST_OFPIT11_WRITE_METADATA:
> > -        parse_metadata(ofpacts, arg);
> > +        error_s = parse_metadata(ofpacts, arg);
> >          break;
> >
> >      case OVSINST_OFPIT11_GOTO_TABLE: {
> >          struct ofpact_goto_table *ogt = ofpact_put_GOTO_TABLE(ofpacts);
> >          char *table_s = strsep(&arg, ",");
> >          if (!table_s || !table_s[0]) {
> > -            ovs_fatal(0, "instruction goto-table needs table id");
> > +            return xstrdup("instruction goto-table needs table id");
> >          }
> > -        ogt->table_id = str_to_u8(table_s, "table");
> > +        error_s = str_to_u8(table_s, "table", &ogt->table_id);
> >          break;
> >      }
> >      }
> >
> > +    if (error_s) {
> > +        return error_s;
> > +    }
> > +
> >      /* If write_metadata is specified as an action AND an instruction,
> > ofpacts
> >         could be invalid. */
> >      error = ofpacts_verify(ofpacts->data, ofpacts->size);
> >      if (error) {
> > -        ovs_fatal(0, "Incorrect instruction ordering");
> > +        return xstrdup("Incorrect instruction ordering");
> >      }
> > +    return NULL;
> >  }
> >
> > -static void
> > +/* Parses 'str' as a series of instructions, and appends them to
> > 'ofpacts'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  str_to_inst_ofpacts(char *str, struct ofpbuf *ofpacts)
> >  {
> > +    size_t orig_size = ofpacts->size;
> >      char *pos, *inst, *arg;
> >      int type;
> >      const char *prev_inst = NULL;
> > @@ -745,8 +975,10 @@ str_to_inst_ofpacts(char *str, struct ofpbuf *ofpacts)
> >      while (ofputil_parse_key_value(&pos, &inst, &arg)) {
> >          type = ovs_instruction_type_from_name(inst);
> >          if (type < 0) {
> > -            if (!str_to_ofpact__(pos, inst, arg, ofpacts, n_actions)) {
> > -                break;
> > +            char *error = str_to_ofpact__(pos, inst, arg, ofpacts,
> > n_actions);
> > +            if (error) {
> > +                ofpacts->size = orig_size;
> > +                return error;
> >              }
> >
> >              type = OVSINST_OFPIT11_APPLY_ACTIONS;
> > @@ -755,19 +987,26 @@ str_to_inst_ofpacts(char *str, struct ofpbuf
> > *ofpacts)
> >                  continue;
> >              }
> >          } else if (type == OVSINST_OFPIT11_APPLY_ACTIONS) {
> > -            ovs_fatal(0, "%s isn't supported. Just write actions then "
> > -                      "it is interpreted as apply_actions", inst);
> > +            ofpacts->size = orig_size;
> > +            return xasprintf("%s isn't supported. Just write actions then
> > "
> > +                             "it is interpreted as apply_actions", inst);
> >          } else {
> > -            parse_named_instruction(type, arg, ofpacts);
> > +            char *error = parse_named_instruction(type, arg, ofpacts);
> > +            if (error) {
> > +                ofpacts->size = orig_size;
> > +                return error;
> > +            }
> >          }
> >
> > -        if (type == prev_type) {
> > -            ovs_fatal(0, "instruction can be specified at most once: %s",
> > -                      inst);
> > -        }
> >          if (type <= prev_type) {
> > -            ovs_fatal(0, "Instruction %s must be specified before %s",
> > -                      inst, prev_inst);
> > +            ofpacts->size = orig_size;
> > +            if (type == prev_type) {
> > +                return xasprintf("instruction %s may be specified only
> > once",
> > +                                 inst);
> > +            } else {
> > +                return xasprintf("instruction %s must be specified before
> > %s",
> > +                                 inst, prev_inst);
> > +            }
> >          }
> >
> >          prev_inst = inst;
> > @@ -775,6 +1014,8 @@ str_to_inst_ofpacts(char *str, struct ofpbuf *ofpacts)
> >          n_actions++;
> >      }
> >      ofpact_pad(ofpacts);
> > +
> > +    return NULL;
> >  }
> >
> >  struct protocol {
> > @@ -813,44 +1054,26 @@ parse_protocol(const char *name, const struct
> > protocol **p_out)
> >      return false;
> >  }
> >
> > -static void
> > -ofp_fatal(const char *flow, bool verbose, const char *format, ...)
> > -{
> > -    va_list args;
> > -
> > -    if (verbose) {
> > -        fprintf(stderr, "%s:\n", flow);
> > -    }
> > -
> > -    va_start(args, format);
> > -    ovs_fatal_valist(0, format, args);
> > -}
> > -
> > -static void
> > +/* Parses 's' as the (possibly masked) value of field 'mf', and updates
> > + * 'match' appropriately.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +static char * WARN_UNUSED_RESULT
> >  parse_field(const struct mf_field *mf, const char *s, struct match *match)
> >  {
> >      union mf_value value, mask;
> >      char *error;
> >
> >      error = mf_parse(mf, s, &value, &mask);
> > -    if (error) {
> > -        ovs_fatal(0, "%s", error);
> > +    if (!error) {
> > +        mf_set(mf, &value, &mask, match);
> >      }
> > -
> > -    mf_set(mf, &value, &mask, match);
> > +    return error;
> >  }
> >
> > -/* Convert 'str_' (as described in the Flow Syntax section of the
> > ovs-ofctl man
> > - * page) into 'fm' for sending the specified flow_mod 'command' to a
> > switch.
> > - * If 'actions' is specified, an action must be in 'string' and may be
> > expanded
> > - * or reallocated.
> > - *
> > - * To parse syntax for an OFPT_FLOW_MOD (or NXT_FLOW_MOD), use an OFPFC_*
> > - * constant for 'command'.  To parse syntax for an OFPST_FLOW or
> > - * OFPST_AGGREGATE (or NXST_FLOW or NXST_AGGREGATE), use -1 for
> > 'command'. */
> > -void
> > -parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_,
> > -              bool verbose)
> > +static char * WARN_UNUSED_RESULT
> > +parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string)
> >  {
> >      enum {
> >          F_OUT_PORT = 1 << 0,
> > @@ -859,7 +1082,6 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int
> > command, const char *str_,
> >          F_PRIORITY = 1 << 4,
> >          F_FLAGS = 1 << 5,
> >      } fields;
> > -    char *string = xstrdup(str_);
> >      char *save_ptr = NULL;
> >      char *act_str = NULL;
> >      char *name;
> > @@ -913,13 +1135,13 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int
> > command, const char *str_,
> >      if (fields & F_ACTIONS) {
> >          act_str = strstr(string, "action");
> >          if (!act_str) {
> > -            ofp_fatal(str_, verbose, "must specify an action");
> > +            return xstrdup("must specify an action");
> >          }
> >          *act_str = '\0';
> >
> >          act_str = strchr(act_str + 1, '=');
> >          if (!act_str) {
> > -            ofp_fatal(str_, verbose, "must specify an action");
> > +            return xstrdup("must specify an action");
> >          }
> >
> >          act_str++;
> > @@ -927,6 +1149,7 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int
> > command, const char *str_,
> >      for (name = strtok_r(string, "=, \t\r\n", &save_ptr); name;
> >           name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) {
> >          const struct protocol *p;
> > +        char *error = NULL;
> >
> >          if (parse_protocol(name, &p)) {
> >              match_set_dl_type(&fm->match, htons(p->dl_type));
> > @@ -948,44 +1171,50 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int
> > command, const char *str_,
> >
> >              value = strtok_r(NULL, ", \t\r\n", &save_ptr);
> >              if (!value) {
> > -                ofp_fatal(str_, verbose, "field %s missing value", name);
> > +                return xasprintf("field %s missing value", name);
> >              }
> >
> >              if (!strcmp(name, "table")) {
> > -                fm->table_id = str_to_u8(value, name);
> > +                error = str_to_u8(value, "table", &fm->table_id);
> >              } else if (!strcmp(name, "out_port")) {
> >                  if (!ofputil_port_from_string(value, &fm->out_port)) {
> > -                    ofp_fatal(str_, verbose, "%s is not a valid OpenFlow
> > port",
> > -                              name);
> > +                    error = xasprintf("%s is not a valid OpenFlow port",
> > +                                      value);
> >                  }
> >              } else if (fields & F_PRIORITY && !strcmp(name, "priority")) {
> > -                fm->priority = str_to_u16(value, name);
> > +                uint16_t priority;
> > +
> > +                error = str_to_u16(value, name, &priority);
> > +                fm->priority = priority;
> >              } else if (fields & F_TIMEOUT && !strcmp(name,
> > "idle_timeout")) {
> > -                fm->idle_timeout = str_to_u16(value, name);
> > +                error = str_to_u16(value, name, &fm->idle_timeout);
> >              } else if (fields & F_TIMEOUT && !strcmp(name,
> > "hard_timeout")) {
> > -                fm->hard_timeout = str_to_u16(value, name);
> > +                error = str_to_u16(value, name, &fm->hard_timeout);
> >              } else if (!strcmp(name, "cookie")) {
> >                  char *mask = strchr(value, '/');
> >
> >                  if (mask) {
> >                      /* A mask means we're searching for a cookie. */
> >                      if (command == OFPFC_ADD) {
> > -                        ofp_fatal(str_, verbose, "flow additions cannot
> > use "
> > -                                  "a cookie mask");
> > +                        return xstrdup("flow additions cannot use "
> > +                                       "a cookie mask");
> >                      }
> >                      *mask = '\0';
> > -                    fm->cookie = htonll(str_to_u64(value));
> > -                    fm->cookie_mask = htonll(str_to_u64(mask+1));
> > +                    error = str_to_be64(value, &fm->cookie);
> > +                    if (error) {
> > +                        return error;
> > +                    }
> > +                    error = str_to_be64(mask + 1, &fm->cookie_mask);
> >                  } else {
> >                      /* No mask means that the cookie is being set. */
> >                      if (command != OFPFC_ADD && command != OFPFC_MODIFY
> > -                            && command != OFPFC_MODIFY_STRICT) {
> > -                        ofp_fatal(str_, verbose, "cannot set cookie");
> > +                        && command != OFPFC_MODIFY_STRICT) {
> > +                        return xstrdup("cannot set cookie");
> >                      }
> > -                    fm->new_cookie = htonll(str_to_u64(value));
> > +                    error = str_to_be64(value, &fm->new_cookie);
> >                  }
> >              } else if (mf_from_name(name)) {
> > -                parse_field(mf_from_name(name), value, &fm->match);
> > +                error = parse_field(mf_from_name(name), value,
> > &fm->match);
> >              } else if (!strcmp(name, "duration")
> >                         || !strcmp(name, "n_packets")
> >                         || !strcmp(name, "n_bytes")
> > @@ -995,12 +1224,16 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int
> > command, const char *str_,
> >                   * "ovs-ofctl dump-flows" back into commands that parse
> >                   * flows. */
> >              } else {
> > -                ofp_fatal(str_, verbose, "unknown keyword %s", name);
> > +                error = xasprintf("unknown keyword %s", name);
> > +            }
> > +
> > +            if (error) {
> > +                return error;
> >              }
> >          }
> >      }
> >      if (!fm->cookie_mask && fm->new_cookie == htonll(UINT64_MAX)
> > -            && (command == OFPFC_MODIFY || command ==
> > OFPFC_MODIFY_STRICT)) {
> > +        && (command == OFPFC_MODIFY || command == OFPFC_MODIFY_STRICT)) {
> >          /* On modifies without a mask, we are supposed to add a flow if
> >           * one does not exist.  If a cookie wasn't been specified, use a
> >           * default of zero. */
> > @@ -1008,40 +1241,69 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int
> > command, const char *str_,
> >      }
> >      if (fields & F_ACTIONS) {
> >          struct ofpbuf ofpacts;
> > -        enum ofperr err;
> > +        char *error;
> >
> >          ofpbuf_init(&ofpacts, 32);
> > -        str_to_inst_ofpacts(act_str, &ofpacts);
> > -        fm->ofpacts_len = ofpacts.size;
> > -        fm->ofpacts = ofpbuf_steal_data(&ofpacts);
> > -
> > -        err = ofpacts_check(fm->ofpacts, fm->ofpacts_len, &fm->match.flow,
> > -                            OFPP_MAX, 0);
> > -        if (err) {
> > -            exit(EXIT_FAILURE);
> > +        error = str_to_inst_ofpacts(act_str, &ofpacts);
> > +        if (!error) {
> > +            enum ofperr err;
> > +
> > +            err = ofpacts_check(ofpacts.data, ofpacts.size,
> > &fm->match.flow,
> > +                                OFPP_MAX, 0);
> > +            if (err) {
> > +                error = xasprintf("actions are invalid with specified
> > match "
> > +                                  "(%s)", ofperr_to_string(err));
> > +            }
> > +        }
> > +        if (error) {
> > +            ofpbuf_uninit(&ofpacts);
> > +            return error;
> >          }
> >
> > +        fm->ofpacts_len = ofpacts.size;
> > +        fm->ofpacts = ofpbuf_steal_data(&ofpacts);
> >      } else {
> >          fm->ofpacts_len = 0;
> >          fm->ofpacts = NULL;
> >      }
> >
> > -    free(string);
> > +    return NULL;
> >  }
> >
> >  /* Convert 'str_' (as described in the Flow Syntax section of the
> > ovs-ofctl man
> > - * page) into 'mm' for sending the specified meter_mod 'command' to a
> > switch.
> > - */
> > -void
> > -parse_ofp_meter_mod_str(struct ofputil_meter_mod *mm, const char *str_,
> > -                        int command, bool verbose)
> > + * page) into 'fm' for sending the specified flow_mod 'command' to a
> > switch.
> > + *
> > + * To parse syntax for an OFPT_FLOW_MOD (or NXT_FLOW_MOD), use an OFPFC_*
> > + * constant for 'command'.  To parse syntax for an OFPST_FLOW or
> > + * OFPST_AGGREGATE (or NXST_FLOW or NXST_AGGREGATE), use -1 for 'command'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> > +parse_ofp_str(struct ofputil_flow_mod *fm, int command, const char *str_)
> > +{
> > +    char *string = xstrdup(str_);
> > +    char *error;
> > +
> > +    error = parse_ofp_str__(fm, command, string);
> > +    if (error) {
> > +        fm->ofpacts = NULL;
> > +        fm->ofpacts_len = 0;
> > +    }
> > +
> > +    free(string);
> > +    return error;
> > +}
> > +
> > +static char * WARN_UNUSED_RESULT
> > +parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, char *string,
> > +                          struct ofpbuf *bands, int command)
> >  {
> >      enum {
> >          F_METER = 1 << 0,
> >          F_FLAGS = 1 << 1,
> >          F_BANDS = 1 << 2,
> >      } fields;
> > -    char *string = xstrdup(str_);
> >      char *save_ptr = NULL;
> >      char *band_str = NULL;
> >      char *name;
> > @@ -1073,13 +1335,13 @@ parse_ofp_meter_mod_str(struct ofputil_meter_mod
> > *mm, const char *str_,
> >      if (fields & F_BANDS) {
> >          band_str = strstr(string, "band");
> >          if (!band_str) {
> > -            ofp_fatal(str_, verbose, "must specify bands");
> > +            return xstrdup("must specify bands");
> >          }
> >          *band_str = '\0';
> >
> >          band_str = strchr(band_str + 1, '=');
> >          if (!band_str) {
> > -            ofp_fatal(str_, verbose, "must specify bands");
> > +            return xstrdup("must specify bands");
> >          }
> >
> >          band_str++;
> > @@ -1100,7 +1362,7 @@ parse_ofp_meter_mod_str(struct ofputil_meter_mod
> > *mm, const char *str_,
> >
> >              value = strtok_r(NULL, ", \t\r\n", &save_ptr);
> >              if (!value) {
> > -                ofp_fatal(str_, verbose, "field %s missing value", name);
> > +                return xasprintf("field %s missing value", name);
> >              }
> >
> >              if (!strcmp(name, "meter")) {
> > @@ -1111,32 +1373,31 @@ parse_ofp_meter_mod_str(struct ofputil_meter_mod
> > *mm, const char *str_,
> >                  } else if (!strcmp(value, "slowpath")) {
> >                      mm->meter.meter_id = OFPM13_SLOWPATH;
> >                  } else {
> > -                    mm->meter.meter_id = str_to_u32(value);
> > +                    char *error = str_to_u32(value, &mm->meter.meter_id);
> > +                    if (error) {
> > +                        return error;
> > +                    }
> >                      if (mm->meter.meter_id > OFPM13_MAX) {
> > -                        ofp_fatal(str_, verbose, "invalid value for %s",
> > name);
> > +                        return xasprintf("invalid value for %s", name);
> >                      }
> >                  }
> >              } else {
> > -                ofp_fatal(str_, verbose, "unknown keyword %s", name);
> > +                return xasprintf("unknown keyword %s", name);
> >              }
> >          }
> >      }
> >      if (fields & F_METER && !mm->meter.meter_id) {
> > -        ofp_fatal(str_, verbose, "must specify 'meter'");
> > +        return xstrdup("must specify 'meter'");
> >      }
> >      if (fields & F_FLAGS && !mm->meter.flags) {
> > -        ofp_fatal(str_, verbose,
> > -                  "meter must specify either 'kbps' or 'pktps'");
> > +        return xstrdup("meter must specify either 'kbps' or 'pktps'");
> >      }
> >
> >      if (fields & F_BANDS) {
> > -        struct ofpbuf bands;
> >          uint16_t n_bands = 0;
> >          struct ofputil_meter_band *band = NULL;
> >          int i;
> >
> > -        ofpbuf_init(&bands, 64);
> > -
> >          for (name = strtok_r(band_str, "=, \t\r\n", &save_ptr); name;
> >               name = strtok_r(NULL, "=, \t\r\n", &save_ptr)) {
> >
> > @@ -1144,12 +1405,12 @@ parse_ofp_meter_mod_str(struct ofputil_meter_mod
> > *mm, const char *str_,
> >
> >              value = strtok_r(NULL, ", \t\r\n", &save_ptr);
> >              if (!value) {
> > -                ofp_fatal(str_, verbose, "field %s missing value", name);
> > +                return xasprintf("field %s missing value", name);
> >              }
> >
> >              if (!strcmp(name, "type")) {
> >                  /* Start a new band */
> > -                band = ofpbuf_put_zeros(&bands, sizeof *band);
> > +                band = ofpbuf_put_zeros(bands, sizeof *band);
> >                  n_bands++;
> >
> >                  if (!strcmp(value, "drop")) {
> > @@ -1157,59 +1418,66 @@ parse_ofp_meter_mod_str(struct ofputil_meter_mod
> > *mm, const char *str_,
> >                  } else if (!strcmp(value, "dscp_remark")) {
> >                      band->type = OFPMBT13_DSCP_REMARK;
> >                  } else {
> > -                    ofp_fatal(str_, verbose, "field %s unknown value %s",
> > name,
> > -                              value);
> > +                    return xasprintf("field %s unknown value %s", name,
> > value);
> >                  }
> >              } else if (!band || !band->type) {
> > -                ofp_fatal(str_, verbose,
> > -                          "band must start with the 'type' keyword");
> > +                return xstrdup("band must start with the 'type' keyword");
> >              } else if (!strcmp(name, "rate")) {
> > -                band->rate = str_to_u32(value);
> > +                char *error = str_to_u32(value, &band->rate);
> > +                if (error) {
> > +                    return error;
> > +                }
> >              } else if (!strcmp(name, "burst_size")) {
> > -                band->burst_size = str_to_u32(value);
> > +                char *error = str_to_u32(value, &band->burst_size);
> > +                if (error) {
> > +                    return error;
> > +                }
> >              } else if (!strcmp(name, "prec_level")) {
> > -                band->prec_level = str_to_u8(value, name);
> > +                char *error = str_to_u8(value, name, &band->prec_level);
> > +                if (error) {
> > +                    return error;
> > +                }
> >              } else {
> > -                ofp_fatal(str_, verbose, "unknown keyword %s", name);
> > +                return xasprintf("unknown keyword %s", name);
> >              }
> >          }
> >          /* validate bands */
> >          if (!n_bands) {
> > -            ofp_fatal(str_, verbose, "meter must have bands");
> > +            return xstrdup("meter must have bands");
> >          }
> >
> >          mm->meter.n_bands = n_bands;
> > -        mm->meter.bands = ofpbuf_steal_data(&bands);
> > +        mm->meter.bands = ofpbuf_steal_data(bands);
> >
> >          for (i = 0; i < n_bands; ++i) {
> >              band = &mm->meter.bands[i];
> >
> >              if (!band->type) {
> > -                ofp_fatal(str_, verbose, "band must have 'type'");
> > +                return xstrdup("band must have 'type'");
> >              }
> >              if (band->type == OFPMBT13_DSCP_REMARK) {
> >                  if (!band->prec_level) {
> > -                    ofp_fatal(str_, verbose, "'dscp_remark' band must
> > have"
> > -                              " 'prec_level'");
> > +                    return xstrdup("'dscp_remark' band must have"
> > +                                   " 'prec_level'");
> >                  }
> >              } else {
> >                  if (band->prec_level) {
> > -                    ofp_fatal(str_, verbose, "Only 'dscp_remark' band may
> > have"
> > -                              " 'prec_level'");
> > +                    return xstrdup("Only 'dscp_remark' band may have"
> > +                                   " 'prec_level'");
> >                  }
> >              }
> >              if (!band->rate) {
> > -                ofp_fatal(str_, verbose, "band must have 'rate'");
> > +                return xstrdup("band must have 'rate'");
> >              }
> >              if (mm->meter.flags & OFPMF13_BURST) {
> >                  if (!band->burst_size) {
> > -                    ofp_fatal(str_, verbose, "band must have 'burst_size'
> > "
> > -                              "when 'burst' flag is set");
> > +                    return xstrdup("band must have 'burst_size' "
> > +                                   "when 'burst' flag is set");
> >                  }
> >              } else {
> >                  if (band->burst_size) {
> > -                    ofp_fatal(str_, verbose, "band may have 'burst_size'
> > only "
> > -                              "when 'burst' flag is set");
> > +                    return xstrdup("band may have 'burst_size' only "
> > +                                   "when 'burst' flag is set");
> >                  }
> >              }
> >          }
> > @@ -1218,18 +1486,39 @@ parse_ofp_meter_mod_str(struct ofputil_meter_mod
> > *mm, const char *str_,
> >          mm->meter.bands = NULL;
> >      }
> >
> > +    return NULL;
> > +}
> > +
> > +/* Convert 'str_' (as described in the Flow Syntax section of the
> > ovs-ofctl man
> > + * page) into 'mm' for sending the specified meter_mod 'command' to a
> > switch.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> > +parse_ofp_meter_mod_str(struct ofputil_meter_mod *mm, const char *str_,
> > +                        int command)
> > +{
> > +    struct ofpbuf bands;
> > +    char *string;
> > +    char *error;
> > +
> > +    ofpbuf_init(&bands, 64);
> > +    string = xstrdup(str_);
> > +
> > +    error = parse_ofp_meter_mod_str__(mm, string, &bands, command);
> > +
> >      free(string);
> > +    ofpbuf_uninit(&bands);
> > +
> > +    return error;
> >  }
> >
> > -/* Convert 'str_' (as described in the documentation for the "monitor"
> > command
> > - * in the ovs-ofctl man page) into 'fmr'. */
> > -void
> > -parse_flow_monitor_request(struct ofputil_flow_monitor_request *fmr,
> > -                           const char *str_)
> > +static char * WARN_UNUSED_RESULT
> > +parse_flow_monitor_request__(struct ofputil_flow_monitor_request *fmr,
> > +                             const char *str_, char *string)
> >  {
> >      static uint32_t id;
> >
> > -    char *string = xstrdup(str_);
> >      char *save_ptr = NULL;
> >      char *name;
> >
> > @@ -1266,95 +1555,162 @@ parse_flow_monitor_request(struct
> > ofputil_flow_monitor_request *fmr,
> >
> >              value = strtok_r(NULL, ", \t\r\n", &save_ptr);
> >              if (!value) {
> > -                ovs_fatal(0, "%s: field %s missing value", str_, name);
> > +                return xasprintf("%s: field %s missing value", str_,
> > name);
> >              }
> >
> >              if (!strcmp(name, "table")) {
> > -                fmr->table_id = str_to_u8(value, name);
> > +                char *error = str_to_u8(value, "table", &fmr->table_id);
> > +                if (error) {
> > +                    return error;
> > +                }
> >              } else if (!strcmp(name, "out_port")) {
> >                  fmr->out_port = u16_to_ofp(atoi(value));
> >              } else if (mf_from_name(name)) {
> > -                parse_field(mf_from_name(name), value, &fmr->match);
> > +                char *error;
> > +
> > +                error = parse_field(mf_from_name(name), value,
> > &fmr->match);
> > +                if (error) {
> > +                    return error;
> > +                }
> >              } else {
> > -                ovs_fatal(0, "%s: unknown keyword %s", str_, name);
> > +                return xasprintf("%s: unknown keyword %s", str_, name);
> >              }
> >          }
> >      }
> > +    return NULL;
> > +}
> > +
> > +/* Convert 'str_' (as described in the documentation for the "monitor"
> > command
> > + * in the ovs-ofctl man page) into 'fmr'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> > +parse_flow_monitor_request(struct ofputil_flow_monitor_request *fmr,
> > +                           const char *str_)
> > +{
> > +    char *string = xstrdup(str_);
> > +    char *error = parse_flow_monitor_request__(fmr, str_, string);
> >      free(string);
> > +    return error;
> >  }
> >
> >  /* Parses 's' as a set of OpenFlow actions and appends the actions to
> >   * 'actions'.
> >   *
> > - * Prints an error on stderr and aborts the program if 's' syntax is
> > - * invalid. */
> > -void
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> >  parse_ofpacts(const char *s_, struct ofpbuf *ofpacts)
> >  {
> >      char *s = xstrdup(s_);
> > -    str_to_ofpacts(s, ofpacts);
> > +    char *error = str_to_ofpacts(s, ofpacts);
> >      free(s);
> > +
> > +    return error;
> >  }
> >
> >  /* Parses 'string' as an OFPT_FLOW_MOD or NXT_FLOW_MOD with command
> > 'command'
> > - * (one of OFPFC_*) into 'fm'. */
> > -void
> > + * (one of OFPFC_*) into 'fm'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> >  parse_ofp_flow_mod_str(struct ofputil_flow_mod *fm, const char *string,
> > -                       uint16_t command, bool verbose)
> > +                       uint16_t command)
> >  {
> > -    struct match match_copy;
> > -
> > -    parse_ofp_str(fm, command, string, verbose);
> > +    char *error = parse_ofp_str(fm, command, string);
> > +    if (!error) {
> > +        /* Normalize a copy of the match.  This ensures that
> > non-normalized
> > +         * flows get logged but doesn't affect what gets sent to the
> > switch, so
> > +         * that the switch can do whatever it likes with the flow. */
> > +        struct match match_copy = fm->match;
> > +        ofputil_normalize_match(&match_copy);
> > +    }
> >
> > -    /* Normalize a copy of the match.  This ensures that non-normalized
> > flows
> > -     * get logged but doesn't affect what gets sent to the switch, so
> > that the
> > -     * switch can do whatever it likes with the flow. */
> > -    match_copy = fm->match;
> > -    ofputil_normalize_match(&match_copy);
> > +    return error;
> >  }
> >
> > -void
> > +/* Opens file 'file_name' and reads each line as a flow_mod of the
> > specified
> > + * type (one of OFPFC_*).  Stores each flow_mod in '*fm', an array
> > allocated
> > + * on the caller's behalf, and the number of flow_mods in '*n_fms'.
> > + *
> > + * Returns NULL if successful, otherwise a malloc()'d string describing
> > the
> > + * error.  The caller is responsible for freeing the returned string. */
> > +char * WARN_UNUSED_RESULT
> >  parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
> >                          struct ofputil_flow_mod **fms, size_t *n_fms)
> >  {
> >      size_t allocated_fms;
> > +    int line_number;
> >      FILE *stream;
> >      struct ds s;
> >
> > +    *fms = NULL;
> > +    *n_fms = 0;
> > +
> >      stream = !strcmp(file_name, "-") ? stdin : fopen(file_name, "r");
> >      if (stream == NULL) {
> > -        ovs_fatal(errno, "%s: open", file_name);
> > +        return xasprintf("%s: open failed (%s)",
> > +                         file_name, ovs_strerror(errno));
> >      }
> >
> >      allocated_fms = *n_fms;
> >      ds_init(&s);
> > -    while (!ds_get_preprocessed_line(&s, stream)) {
> > +    line_number = 0;
> > +    while (!ds_get_preprocessed_line(&s, stream, &line_number)) {
> > +        char *error;
> > +
> >          if (*n_fms >= allocated_fms) {
> >              *fms = x2nrealloc(*fms, &allocated_fms, sizeof **fms);
> >          }
> > -        parse_ofp_flow_mod_str(&(*fms)[*n_fms], ds_cstr(&s), command,
> > false);
> > +        error = parse_ofp_flow_mod_str(&(*fms)[*n_fms], ds_cstr(&s),
> > command);
> > +        if (error) {
> > +            size_t i;
> > +
> > +            for (i = 0; i < *n_fms; i++) {
> > +                free((*fms)[i].ofpacts);
> > +            }
> > +            free(*fms);
> > +            *fms = NULL;
> > +            *n_fms = 0;
> > +
> > +            ds_destroy(&s);
> > +            if (stream != stdin) {
> > +                fclose(stream);
> > +            }
> > +
> > +            return xasprintf("%s:%d: %s", file_name, line_number, error);
> > +        }
> >          *n_fms += 1;
> >      }
> > -    ds_destroy(&s);
> >
> > +    ds_destroy(&s);
> >      if (stream != stdin) {
> >          fclose(stream);
> >      }
> > +    return NULL;
> >  }
> >
> > -void
> > +char * WARN_UNUSED_RESULT
> >  parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request *fsr,
> >                                   bool aggregate, const char *string)
> >  {
> >      struct ofputil_flow_mod fm;
> > +    char *error;
> > +
> > +    error = parse_ofp_str(&fm, -1, string);
> > +    if (error) {
> > +        return error;
> > +    }
> >
> > -    parse_ofp_str(&fm, -1, string, false);
> >      fsr->aggregate = aggregate;
> >      fsr->cookie = fm.cookie;
> >      fsr->cookie_mask = fm.cookie_mask;
> >      fsr->match = fm.match;
> >      fsr->out_port = fm.out_port;
> >      fsr->table_id = fm.table_id;
> > +    return NULL;
> >  }
> >
> >  /* Parses a specification of a flow from 's' into 'flow'.  's' must take
> > the
> > diff --git a/lib/ofp-parse.h b/lib/ofp-parse.h
> > index 6ee25a4..707f5d8 100644
> > --- a/lib/ofp-parse.h
> > +++ b/lib/ofp-parse.h
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -22,6 +22,7 @@
> >  #include <stdbool.h>
> >  #include <stdint.h>
> >  #include <stdio.h>
> > +#include "compiler.h"
> >
> >  struct flow;
> >  struct ofpbuf;
> > @@ -30,26 +31,31 @@ struct ofputil_flow_monitor_request;
> >  struct ofputil_flow_stats_request;
> >  struct ofputil_meter_mod;
> >
> > -void parse_ofp_str(struct ofputil_flow_mod *, int command, const char
> > *str_,
> > -                   bool verbose);
> > +char *parse_ofp_str(struct ofputil_flow_mod *, int command, const char
> > *str_)
> > +    WARN_UNUSED_RESULT;
> >
> > -void parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string,
> > -                            uint16_t command, bool verbose);
> > -void parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
> > -                             struct ofputil_flow_mod **fms, size_t
> > *n_fms);
> > +char *parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char
> > *string,
> > +                            uint16_t command)
> > +    WARN_UNUSED_RESULT;
> > +char *parse_ofp_flow_mod_file(const char *file_name, uint16_t command,
> > +                              struct ofputil_flow_mod **fms, size_t
> > *n_fms)
> > +    WARN_UNUSED_RESULT;
> >
> > -void parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request *,
> > -                                      bool aggregate, const char *string);
> > +char *parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request
> > *,
> > +                                       bool aggregate, const char *string)
> > +    WARN_UNUSED_RESULT;
> >
> > -
> > -void parse_ofpacts(const char *, struct ofpbuf *ofpacts);
> > +char *parse_ofpacts(const char *, struct ofpbuf *ofpacts)
> > +    WARN_UNUSED_RESULT;
> >
> >  char *parse_ofp_exact_flow(struct flow *, const char *);
> >
> > -void parse_ofp_meter_mod_str(struct ofputil_meter_mod *, const char
> > *string,
> > -                             int command, bool verbose);
> > +char *parse_ofp_meter_mod_str(struct ofputil_meter_mod *, const char
> > *string,
> > +                             int command)
> > +    WARN_UNUSED_RESULT;
> >
> > -void parse_flow_monitor_request(struct ofputil_flow_monitor_request *,
> > -                                const char *);
> > +char *parse_flow_monitor_request(struct ofputil_flow_monitor_request *,
> > +                                const char *)
> > +    WARN_UNUSED_RESULT;
> >
> >  #endif /* ofp-parse.h */
> > diff --git a/tests/learn.at b/tests/learn.at
> > index ec1c347..ce810b4 100644
> > --- a/tests/learn.at
> > +++ b/tests/learn.at
> > @@ -75,11 +75,13 @@ AT_CHECK([[ovs-ofctl parse-flow
> > 'actions=learn(load:5->NXM_OF_IP_DST[])']],
> >    [1], [], [stderr])
> >  AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0],
> >    [[destination field ip_dst lacks correct prerequisites
> > +ovs-ofctl: actions are invalid with specified match (OFPBAC_BAD_ARGUMENT)
> >  ]], [[]])
> >  AT_CHECK([[ovs-ofctl parse-flow
> > 'actions=learn(load:NXM_OF_IP_DST[]->NXM_NX_REG1[])']],
> >    [1], [], [stderr])
> >  AT_CHECK([sed -e 's/.*|meta_flow|WARN|//' < stderr], [0],
> >    [[source field ip_dst lacks correct prerequisites
> > +ovs-ofctl: actions are invalid with specified match (OFPBAC_BAD_ARGUMENT)
> >  ]])
> >  AT_CLEANUP
> >
> > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> > index 88492cf..5e2afb9 100644
> > --- a/tests/ofproto-dpif.at
> > +++ b/tests/ofproto-dpif.at
> > @@ -166,7 +166,7 @@ AT_SETUP([ofproto-dpif - DSCP])
> >  OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy])
> >  ADD_OF_PORTS([br0], [9])
> >  AT_DATA([flows.txt], [dnl
> >
> > -actions=output:65534,enqueue:1:1,enqueue:1:2,enqueue:1:2,enqueue:1:1,output:1,mod_nw_tos:0,output:1,output:65534
> >
> > +actions=output:LOCAL,enqueue:1:1,enqueue:1:2,enqueue:1:2,enqueue:1:1,output:1,mod_nw_tos:0,output:1,output:LOCAL
> >  ])
> >  AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >  AT_CHECK([ovs-vsctl -- \
> > @@ -197,8 +197,8 @@ AT_DATA([flows.txt], [dnl
> >  in_port=local actions=local,flood
> >  in_port=1 actions=flood
> >  in_port=2 actions=all
> > -in_port=3
> > actions=output:65534,output:1,output:2,output:3,output:4,output:5,output:6,output:7
> > -in_port=4
> > actions=enqueue:65534:1,enqueue:1:1,enqueue:2:1,enqueue:3:2,enqueue:4:1,enqueue:5:1,enqueue:6:1,enqueue:7:1
> > +in_port=3
> > actions=output:LOCAL,output:1,output:2,output:3,output:4,output:5,output:6,output:7
> > +in_port=4
> > actions=enqueue:LOCAL:1,enqueue:1:1,enqueue:2:1,enqueue:3:2,enqueue:4:1,enqueue:5:1,enqueue:6:1,enqueue:7:1
> >  ])
> >  AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >  AT_CHECK([ovs-ofctl mod-port br0 5 noforward])
> > diff --git a/tests/test-bundle.c b/tests/test-bundle.c
> > index 0e7525c..41e2e38 100644
> > --- a/tests/test-bundle.c
> > +++ b/tests/test-bundle.c
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2011, 2012 Nicira, Inc.
> > +/* Copyright (c) 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -71,9 +71,14 @@ parse_bundle_actions(char *actions)
> >      struct ofpact_bundle *bundle;
> >      struct ofpbuf ofpacts;
> >      struct ofpact *action;
> > +    char *error;
> >
> >      ofpbuf_init(&ofpacts, 0);
> > -    bundle_parse_load(actions, &ofpacts);
> > +    error = bundle_parse_load(actions, &ofpacts);
> > +    if (error) {
> > +        ovs_fatal(0, "%s", error);
> > +    }
> > +
> >      action = ofpacts.data;
> >      bundle = ofpact_get_BUNDLE(xmemdup(action, action->len));
> >      ofpbuf_uninit(&ofpacts);
> > diff --git a/tests/test-multipath.c b/tests/test-multipath.c
> > index e8aacff..f1b12e2 100644
> > --- a/tests/test-multipath.c
> > +++ b/tests/test-multipath.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2012 Nicira, Inc.
> > + * Copyright (c) 2010, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -35,6 +35,7 @@ main(int argc, char *argv[])
> >      enum { MP_MAX_LINKS = 63 };
> >      struct ofpact_multipath mp;
> >      bool ok = true;
> > +    char *error;
> >      int n;
> >
> >      set_program_name(argv[0]);
> > @@ -44,7 +45,11 @@ main(int argc, char *argv[])
> >          ovs_fatal(0, "usage: %s multipath_action", program_name);
> >      }
> >
> > -    multipath_parse(&mp, argv[1]);
> > +    error = multipath_parse(&mp, argv[1]);
> > +    if (error) {
> > +        ovs_fatal(0, "%s", error);
> > +    }
> > +
> >      for (n = 1; n <= MP_MAX_LINKS; n++) {
> >          enum { N_FLOWS = 65536 };
> >          double disruption, perfect, distribution;
> > diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c
> > index 6d08af5..d9ae983 100644
> > --- a/utilities/ovs-controller.c
> > +++ b/utilities/ovs-controller.c
> > @@ -278,6 +278,7 @@ parse_options(int argc, char *argv[])
> >
> >      for (;;) {
> >          int indexptr;
> > +        char *error;
> >          int c;
> >
> >          c = getopt_long(argc, argv, short_options, long_options,
> > &indexptr);
> > @@ -327,8 +328,11 @@ parse_options(int argc, char *argv[])
> >              break;
> >
> >          case OPT_WITH_FLOWS:
> > -            parse_ofp_flow_mod_file(optarg, OFPFC_ADD, &default_flows,
> > -                                    &n_default_flows);
> > +            error = parse_ofp_flow_mod_file(optarg, OFPFC_ADD,
> > &default_flows,
> > +                                            &n_default_flows);
> > +            if (error) {
> > +                ovs_fatal(0, "%s", error);
> > +            }
> >              break;
> >
> >          case OPT_UNIXCTL:
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index adf86ad..6453167 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -866,8 +866,14 @@ prepare_dump_flows(int argc, char *argv[], bool
> > aggregate,
> >      enum ofputil_protocol usable_protocols, protocol;
> >      struct ofputil_flow_stats_request fsr;
> >      struct vconn *vconn;
> > +    char *error;
> > +
> > +    error = parse_ofp_flow_stats_request_str(&fsr, aggregate,
> > +                                             argc > 2 ? argv[2] : "");
> > +    if (error) {
> > +        ovs_fatal(0, "%s", error);
> > +    }
> >
> > -    parse_ofp_flow_stats_request_str(&fsr, aggregate, argc > 2 ? argv[2]
> > : "");
> >      usable_protocols = ofputil_flow_stats_request_usable_protocols(&fsr);
> >
> >      protocol = open_vconn(argv[1], &vconn);
> > @@ -1087,8 +1093,12 @@ ofctl_flow_mod_file(int argc OVS_UNUSED, char
> > *argv[], uint16_t command)
> >  {
> >      struct ofputil_flow_mod *fms = NULL;
> >      size_t n_fms = 0;
> > +    char *error;
> >
> > -    parse_ofp_flow_mod_file(argv[2], command, &fms, &n_fms);
> > +    error = parse_ofp_flow_mod_file(argv[2], command, &fms, &n_fms);
> > +    if (error) {
> > +        ovs_fatal(0, "%s", error);
> > +    }
> >      ofctl_flow_mod__(argv[1], fms, n_fms);
> >      free(fms);
> >  }
> > @@ -1100,7 +1110,12 @@ ofctl_flow_mod(int argc, char *argv[], uint16_t
> > command)
> >          ofctl_flow_mod_file(argc, argv, command);
> >      } else {
> >          struct ofputil_flow_mod fm;
> > -        parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "", command,
> > false);
> > +        char *error;
> > +
> > +        error = parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "",
> > command);
> > +        if (error) {
> > +            ovs_fatal(0, "%s", error);
> > +        }
> >          ofctl_flow_mod__(argv[1], &fm, 1);
> >      }
> >  }
> > @@ -1439,8 +1454,12 @@ ofctl_monitor(int argc, char *argv[])
> >          } else if (!strncmp(arg, "watch:", 6)) {
> >              struct ofputil_flow_monitor_request fmr;
> >              struct ofpbuf *msg;
> > +            char *error;
> >
> > -            parse_flow_monitor_request(&fmr, arg + 6);
> > +            error = parse_flow_monitor_request(&fmr, arg + 6);
> > +            if (error) {
> > +                ovs_fatal(0, "%s", error);
> > +            }
> >
> >              msg = ofpbuf_new(0);
> >              ofputil_append_flow_monitor_request(&fmr, msg);
> > @@ -1538,10 +1557,14 @@ ofctl_packet_out(int argc, char *argv[])
> >      struct ofputil_packet_out po;
> >      struct ofpbuf ofpacts;
> >      struct vconn *vconn;
> > +    char *error;
> >      int i;
> >
> >      ofpbuf_init(&ofpacts, 64);
> > -    parse_ofpacts(argv[3], &ofpacts);
> > +    error = parse_ofpacts(argv[3], &ofpacts);
> > +    if (error) {
> > +        ovs_fatal(0, "%s", error);
> > +    }
> >
> >      po.buffer_id = UINT32_MAX;
> >      po.in_port = str_to_port_no(argv[1], argv[2]);
> > @@ -1905,6 +1928,7 @@ static enum ofputil_protocol
> >  read_flows_from_file(const char *filename, struct classifier *cls, int
> > index)
> >  {
> >      enum ofputil_protocol usable_protocols;
> > +    int line_number;
> >      struct ds s;
> >      FILE *file;
> >
> > @@ -1915,11 +1939,16 @@ read_flows_from_file(const char *filename, struct
> > classifier *cls, int index)
> >
> >      ds_init(&s);
> >      usable_protocols = OFPUTIL_P_ANY;
> > -    while (!ds_get_preprocessed_line(&s, file)) {
> > +    line_number = 0;
> > +    while (!ds_get_preprocessed_line(&s, file, &line_number)) {
> >          struct fte_version *version;
> >          struct ofputil_flow_mod fm;
> > +        char *error;
> >
> > -        parse_ofp_str(&fm, OFPFC_ADD, ds_cstr(&s), true);
> > +        error = parse_ofp_str(&fm, OFPFC_ADD, ds_cstr(&s));
> > +        if (error) {
> > +            ovs_fatal(0, "%s:%d: %s", filename, line_number, error);
> > +        }
> >
> >          version = xmalloc(sizeof *version);
> >          version->cookie = fm.new_cookie;
> > @@ -2232,8 +2261,12 @@ static void
> >  ofctl_parse_flow(int argc OVS_UNUSED, char *argv[])
> >  {
> >      struct ofputil_flow_mod fm;
> > +    char *error;
> >
> > -    parse_ofp_flow_mod_str(&fm, argv[1], OFPFC_ADD, false);
> > +    error = parse_ofp_flow_mod_str(&fm, argv[1], OFPFC_ADD);
> > +    if (error) {
> > +        ovs_fatal(0, "%s", error);
> > +    }
> >      ofctl_parse_flows__(&fm, 1);
> >  }
> >
> > @@ -2244,8 +2277,12 @@ ofctl_parse_flows(int argc OVS_UNUSED, char *argv[])
> >  {
> >      struct ofputil_flow_mod *fms = NULL;
> >      size_t n_fms = 0;
> > +    char *error;
> >
> > -    parse_ofp_flow_mod_file(argv[1], OFPFC_ADD, &fms, &n_fms);
> > +    error = parse_ofp_flow_mod_file(argv[1], OFPFC_ADD, &fms, &n_fms);
> > +    if (error) {
> > +        ovs_fatal(0, "%s", error);
> > +    }
> >      ofctl_parse_flows__(fms, n_fms);
> >      free(fms);
> >  }
> > @@ -2367,7 +2404,7 @@ ofctl_parse_ofp10_actions(int argc OVS_UNUSED, char
> > *argv[] OVS_UNUSED)
> >      struct ds in;
> >
> >      ds_init(&in);
> > -    while (!ds_get_preprocessed_line(&in, stdin)) {
> > +    while (!ds_get_preprocessed_line(&in, stdin, NULL)) {
> >          struct ofpbuf of10_out;
> >          struct ofpbuf of10_in;
> >          struct ofpbuf ofpacts;
> > @@ -2431,7 +2468,7 @@ ofctl_parse_ofp10_match(int argc OVS_UNUSED, char
> > *argv[] OVS_UNUSED)
> >
> >      ds_init(&in);
> >      ds_init(&expout);
> > -    while (!ds_get_preprocessed_line(&in, stdin)) {
> > +    while (!ds_get_preprocessed_line(&in, stdin, NULL)) {
> >          struct ofpbuf match_in, match_expout;
> >          struct ofp10_match match_out;
> >          struct ofp10_match match_normal;
> > @@ -2503,7 +2540,7 @@ ofctl_parse_ofp11_match(int argc OVS_UNUSED, char
> > *argv[] OVS_UNUSED)
> >      struct ds in;
> >
> >      ds_init(&in);
> > -    while (!ds_get_preprocessed_line(&in, stdin)) {
> > +    while (!ds_get_preprocessed_line(&in, stdin, NULL)) {
> >          struct ofpbuf match_in;
> >          struct ofp11_match match_out;
> >          struct match match;
> > @@ -2552,7 +2589,7 @@ ofctl_parse_ofp11_actions(int argc OVS_UNUSED, char
> > *argv[] OVS_UNUSED)
> >      struct ds in;
> >
> >      ds_init(&in);
> > -    while (!ds_get_preprocessed_line(&in, stdin)) {
> > +    while (!ds_get_preprocessed_line(&in, stdin, NULL)) {
> >          struct ofpbuf of11_out;
> >          struct ofpbuf of11_in;
> >          struct ofpbuf ofpacts;
> > @@ -2610,7 +2647,7 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED,
> > char *argv[] OVS_UNUSED)
> >      struct ds in;
> >
> >      ds_init(&in);
> > -    while (!ds_get_preprocessed_line(&in, stdin)) {
> > +    while (!ds_get_preprocessed_line(&in, stdin, NULL)) {
> >          struct ofpbuf of11_out;
> >          struct ofpbuf of11_in;
> >          struct ofpbuf ofpacts;
> > @@ -2699,6 +2736,7 @@ ofctl_check_vlan(int argc OVS_UNUSED, char *argv[])
> >      struct match of11_match;
> >
> >      enum ofperr error;
> > +    char *error_s;
> >
> >      match_init_catchall(&match);
> >      match.flow.vlan_tci = htons(strtoul(argv[1], NULL, 16));
> > @@ -2708,7 +2746,10 @@ ofctl_check_vlan(int argc OVS_UNUSED, char *argv[])
> >      string_s = match_to_string(&match, OFP_DEFAULT_PRIORITY);
> >      printf("%s -> ", string_s);
> >      fflush(stdout);
> > -    parse_ofp_str(&fm, -1, string_s, false);
> > +    error_s = parse_ofp_str(&fm, -1, string_s);
> > +    if (error_s) {
> > +        ovs_fatal(0, "%s", error_s);
> > +    }
> >      printf("%04"PRIx16"/%04"PRIx16"\n",
> >             ntohs(fm.match.flow.vlan_tci),
> >             ntohs(fm.match.wc.masks.vlan_tci));
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >



More information about the dev mailing list