Message ID | 1503701479-43894-8-git-send-email-yihung.wei@gmail.com |
---|---|
State | Deferred |
Headers | show |
On 08/25/2017 03:51 PM, Yi-Hung Wei wrote: > Refactor parse_ct_state() to support different delimiters. Why? I don't see any use of different delimiters in this patch and if subsequent patches do use different delimiters then that should be explained here I think. > > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> > --- > lib/flow.c | 6 +++--- > lib/flow.h | 2 +- > ofproto/ofproto-dpif-trace.c | 2 +- > ovn/utilities/ovn-trace.c | 2 +- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/lib/flow.c b/lib/flow.c > index b2b10aa488be..34bc176e8b6e 100644 > --- a/lib/flow.c > +++ b/lib/flow.c > @@ -1129,14 +1129,14 @@ ct_state_from_string(const char *s) > * returns false, and reports error message in 'ds'. */ > bool > parse_ct_state(const char *state_str, uint32_t default_state, > - uint32_t *ct_state, struct ds *ds) > + const char *delimiters, uint32_t *ct_state, struct ds *ds) > { > uint32_t state = default_state; > char *state_s = xstrdup(state_str); > char *save_ptr = NULL; > > - for (char *cs = strtok_r(state_s, ", ", &save_ptr); cs; > - cs = strtok_r(NULL, ", ", &save_ptr)) { > + for (char *cs = strtok_r(state_s, delimiters, &save_ptr); cs; > + cs = strtok_r(NULL, delimiters, &save_ptr)) { > uint32_t bit = ct_state_from_string(cs); > if (!bit) { > ds_put_format(ds, "%s: unknown connection tracking state flag", > diff --git a/lib/flow.h b/lib/flow.h > index 6ae5a674df46..d1aba31290f7 100644 > --- a/lib/flow.h > +++ b/lib/flow.h > @@ -76,7 +76,7 @@ void flow_get_metadata(const struct flow *, struct match *flow_metadata); > const char *ct_state_to_string(uint32_t state); > uint32_t ct_state_from_string(const char *); > bool parse_ct_state(const char *state_str, uint32_t default_state, > - uint32_t *ct_state, struct ds *); > + const char *delimiters, uint32_t *ct_state, struct ds *); > bool validate_ct_state(uint32_t state, struct ds *); > void flow_clear_conntrack(struct flow *); > > diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c > index a86cf211803e..9303ea18c237 100644 > --- a/ofproto/ofproto-dpif-trace.c > +++ b/ofproto/ofproto-dpif-trace.c > @@ -199,7 +199,7 @@ parse_oftrace_options(int argc, const char *argv[], > } > > uint32_t ct_state; > - if (!parse_ct_state(argv[++k], 0, &ct_state, &ds)) { > + if (!parse_ct_state(argv[++k], 0, ", ", &ct_state, &ds)) { > return ds_steal_cstr(&ds); > } > if (!validate_ct_state(ct_state, &ds)) { > diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c > index 59083eebe270..9f4cbf58d3e6 100644 > --- a/ovn/utilities/ovn-trace.c > +++ b/ovn/utilities/ovn-trace.c > @@ -173,7 +173,7 @@ parse_ct_option(const char *state_s_) > uint32_t state; > struct ds ds = DS_EMPTY_INITIALIZER; > > - if (!parse_ct_state(state_s_, CS_TRACKED, &state, &ds)) { > + if (!parse_ct_state(state_s_, CS_TRACKED, ", ", &state, &ds)) { > ovs_fatal(0, "%s", ds_cstr(&ds)); > } > if (!validate_ct_state(state, &ds)) { > Other than my comment above this patch looks fine. Reviewed-by: Greg Rose <gvrose8192@gmail.com>
On Fri, Aug 25, 2017 at 03:51:17PM -0700, Yi-Hung Wei wrote: > Refactor parse_ct_state() to support different delimiters. > > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> > --- > lib/flow.c | 6 +++--- > lib/flow.h | 2 +- > ofproto/ofproto-dpif-trace.c | 2 +- > ovn/utilities/ovn-trace.c | 2 +- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/lib/flow.c b/lib/flow.c > index b2b10aa488be..34bc176e8b6e 100644 > --- a/lib/flow.c > +++ b/lib/flow.c > @@ -1129,14 +1129,14 @@ ct_state_from_string(const char *s) > * returns false, and reports error message in 'ds'. */ > bool > parse_ct_state(const char *state_str, uint32_t default_state, > - uint32_t *ct_state, struct ds *ds) > + const char *delimiters, uint32_t *ct_state, struct ds *ds) Thanks for working on this. parse_ct_state() has a pretty good function-level comment, so would you mind updating it to mention the new parameter? Thanks, Ben.
diff --git a/lib/flow.c b/lib/flow.c index b2b10aa488be..34bc176e8b6e 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -1129,14 +1129,14 @@ ct_state_from_string(const char *s) * returns false, and reports error message in 'ds'. */ bool parse_ct_state(const char *state_str, uint32_t default_state, - uint32_t *ct_state, struct ds *ds) + const char *delimiters, uint32_t *ct_state, struct ds *ds) { uint32_t state = default_state; char *state_s = xstrdup(state_str); char *save_ptr = NULL; - for (char *cs = strtok_r(state_s, ", ", &save_ptr); cs; - cs = strtok_r(NULL, ", ", &save_ptr)) { + for (char *cs = strtok_r(state_s, delimiters, &save_ptr); cs; + cs = strtok_r(NULL, delimiters, &save_ptr)) { uint32_t bit = ct_state_from_string(cs); if (!bit) { ds_put_format(ds, "%s: unknown connection tracking state flag", diff --git a/lib/flow.h b/lib/flow.h index 6ae5a674df46..d1aba31290f7 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -76,7 +76,7 @@ void flow_get_metadata(const struct flow *, struct match *flow_metadata); const char *ct_state_to_string(uint32_t state); uint32_t ct_state_from_string(const char *); bool parse_ct_state(const char *state_str, uint32_t default_state, - uint32_t *ct_state, struct ds *); + const char *delimiters, uint32_t *ct_state, struct ds *); bool validate_ct_state(uint32_t state, struct ds *); void flow_clear_conntrack(struct flow *); diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c index a86cf211803e..9303ea18c237 100644 --- a/ofproto/ofproto-dpif-trace.c +++ b/ofproto/ofproto-dpif-trace.c @@ -199,7 +199,7 @@ parse_oftrace_options(int argc, const char *argv[], } uint32_t ct_state; - if (!parse_ct_state(argv[++k], 0, &ct_state, &ds)) { + if (!parse_ct_state(argv[++k], 0, ", ", &ct_state, &ds)) { return ds_steal_cstr(&ds); } if (!validate_ct_state(ct_state, &ds)) { diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c index 59083eebe270..9f4cbf58d3e6 100644 --- a/ovn/utilities/ovn-trace.c +++ b/ovn/utilities/ovn-trace.c @@ -173,7 +173,7 @@ parse_ct_option(const char *state_s_) uint32_t state; struct ds ds = DS_EMPTY_INITIALIZER; - if (!parse_ct_state(state_s_, CS_TRACKED, &state, &ds)) { + if (!parse_ct_state(state_s_, CS_TRACKED, ", ", &state, &ds)) { ovs_fatal(0, "%s", ds_cstr(&ds)); } if (!validate_ct_state(state, &ds)) {
Refactor parse_ct_state() to support different delimiters. Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> --- lib/flow.c | 6 +++--- lib/flow.h | 2 +- ofproto/ofproto-dpif-trace.c | 2 +- ovn/utilities/ovn-trace.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-)