Looks good.
Ethan
On Fri, Aug 19, 2011 at 15:28, Ben Pfaff <b...@nicira.com> wrote:
> This will soon have a new user, but it's a worthwhile cleanup on its own.
> ---
> lib/ofp-parse.c | 63 ++++---------------------------------------------
> lib/ofp-util.c | 69
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/ofp-util.h | 3 ++
> 3 files changed, 78 insertions(+), 57 deletions(-)
>
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index 7e38565..77d3f19 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -495,80 +495,29 @@ parse_named_action(enum ofputil_action_code code,
> struct ofpbuf *b, char *arg)
> static void
> str_to_action(char *str, struct ofpbuf *b)
> {
> - bool drop = false;
> + char *pos, *act, *arg;
> int n_actions;
> - char *pos;
>
> pos = str;
> n_actions = 0;
> - for (;;) {
> - char empty_string[] = "";
> - char *act, *arg;
> - size_t actlen;
> + while (ofputil_parse_key_value(&pos, &act, &arg)) {
> uint16_t port;
> int code;
>
> - pos += strspn(pos, ", \t\r\n");
> - if (*pos == '\0') {
> - break;
> - }
> -
> - if (drop) {
> - ovs_fatal(0, "Drop actions must not be followed by other
> actions");
> - }
> -
> - act = pos;
> - actlen = strcspn(pos, ":(, \t\r\n");
> - if (act[actlen] == ':') {
> - /* The argument can be separated by a colon. */
> - size_t arglen;
> -
> - arg = act + actlen + 1;
> - arglen = strcspn(arg, ", \t\r\n");
> - pos = arg + arglen + (arg[arglen] != '\0');
> - arg[arglen] = '\0';
> - } else if (act[actlen] == '(') {
> - /* The argument can be surrounded by balanced parentheses. The
> - * outermost set of parentheses is removed. */
> - int level = 1;
> - size_t arglen;
> -
> - arg = act + actlen + 1;
> - for (arglen = 0; level > 0; arglen++) {
> - switch (arg[arglen]) {
> - case '\0':
> - ovs_fatal(0, "unbalanced parentheses in argument to %s "
> - "action", act);
> -
> - case '(':
> - level++;
> - break;
> -
> - case ')':
> - level--;
> - break;
> - }
> - }
> - arg[arglen - 1] = '\0';
> - pos = arg + arglen;
> - } else {
> - /* There might be no argument at all. */
> - arg = empty_string;
> - pos = act + actlen + (act[actlen] != '\0');
> - }
> - act[actlen] = '\0';
> -
> code = ofputil_action_code_from_name(act);
> if (code >= 0) {
> parse_named_action(code, b, arg);
> } else if (!strcasecmp(act, "drop")) {
> /* A drop action in OpenFlow occurs by just not setting
> * an action. */
> - drop = true;
> if (n_actions) {
> ovs_fatal(0, "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");
> }
> + break;
> } else if (!strcasecmp(act, "CONTROLLER")) {
> struct ofp_action_output *oao;
> oao = put_output_action(b, OFPP_CONTROLLER);
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 5cc1d40..a97a0e3 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -2735,3 +2735,72 @@ ofputil_actions_clone(const union ofp_action *actions,
> size_t n)
> {
> return n ? xmemdup(actions, n * sizeof *actions) : NULL;
> }
> +
> +/* Parses a key or a key-value pair from '*stringp'.
> + *
> + * On success: Stores the key into '*keyp'. Stores the value, if present,
> into
> + * '*valuep', otherwise an empty string. Advances '*stringp' past the end of
> + * the key-value pair, preparing it for another call. '*keyp' and '*valuep'
> + * are substrings of '*stringp' created by replacing some of its bytes by
> null
> + * terminators. Returns true.
> + *
> + * If '*stringp' is just white space or commas, sets '*keyp' and '*valuep' to
> + * NULL and returns false. */
> +bool
> +ofputil_parse_key_value(char **stringp, char **keyp, char **valuep)
> +{
> + char *pos, *key, *value;
> + size_t key_len;
> +
> + pos = *stringp;
> + pos += strspn(pos, ", \t\r\n");
> + if (*pos == '\0') {
> + *keyp = *valuep = NULL;
> + return false;
> + }
> +
> + key = pos;
> + key_len = strcspn(pos, ":=(, \t\r\n");
> + if (key[key_len] == ':' || key[key_len] == '=') {
> + /* The value can be separated by a colon. */
> + size_t value_len;
> +
> + value = key + key_len + 1;
> + value_len = strcspn(value, ", \t\r\n");
> + pos = value + value_len + (value[value_len] != '\0');
> + value[value_len] = '\0';
> + } else if (key[key_len] == '(') {
> + /* The value can be surrounded by balanced parentheses. The
> outermost
> + * set of parentheses is removed. */
> + int level = 1;
> + size_t value_len;
> +
> + value = key + key_len + 1;
> + for (value_len = 0; level > 0; value_len++) {
> + switch (value[value_len]) {
> + case '\0':
> + ovs_fatal(0, "unbalanced parentheses in argument to %s",
> key);
> +
> + case '(':
> + level++;
> + break;
> +
> + case ')':
> + level--;
> + break;
> + }
> + }
> + value[value_len - 1] = '\0';
> + pos = value + value_len;
> + } else {
> + /* There might be no value at all. */
> + value = key + key_len; /* Will become the empty string below. */
> + pos = key + key_len + (key[key_len] != '\0');
> + }
> + key[key_len] = '\0';
> +
> + *stringp = pos;
> + *keyp = key;
> + *valuep = value;
> + return true;
> +}
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index a658df1..ecd77cc 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -547,4 +547,7 @@ int ofputil_decode_error_msg(const struct ofp_header *,
> size_t *payload_ofs);
> void ofputil_format_error(struct ds *, int error);
> char *ofputil_error_to_string(int error);
>
> +/* Handy utility for parsing flows and actions. */
> +bool ofputil_parse_key_value(char **stringp, char **keyp, char **valuep);
> +
> #endif /* ofp-util.h */
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
