| Message ID | 8a1463c223497fca2fd3f11a54db5d7e52d1d08a.1695330852.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | config-parse: create config parsing library | expand |
Josh Steadmon <steadmon@google.com> writes: > +static void start_event(struct config_source *cs, enum config_event_t type, > + struct parse_event_data *data) > +{ > + data->previous_type = type; > + data->previous_offset = get_corrected_offset(cs, type); > +} It's a pity that get_corrected_offset() has to be called twice (once here and once below) but I think that's the best we can do given how the code is laid out (and I can't think of a better code layout either). > +static int flush_event(struct config_source *cs, enum config_event_t type, > + struct parse_event_data *data) One thing confusing here is that the "type" is not what's being flushed, but used to change details about how we flush. Technically all we need is is_whitespace_type and is_eof_type, but that's clumsier to code. I think the best we can do is add some documentation to this function, maybe 'Flush the event started by a prior start_event(), if one exists. The type of the event being flushed is not "type" but the type that was passed to the prior start_event(); "type" here may merely change how the flush is performed' or something like that. > +{ > + if (!data->opts || !data->opts->event_fn) > + return 0; > + > + if (type == CONFIG_EVENT_WHITESPACE && > + data->previous_type == type) > + return 0; > > if (data->previous_type != CONFIG_EVENT_EOF && > data->opts->event_fn(data->previous_type, data->previous_offset, > - offset, cs, data->opts->event_fn_data) < 0) > + get_corrected_offset(cs, type), cs, > + data->opts->event_fn_data) < 0) > return -1; Another confusing point here is how EOF is used both to mean "start_event() was never called" and a true EOF. I think for now it's best to just document this where we define CONFIG_EVENT_EOF.
diff --git a/config.c b/config.c index 1518f70fc2..ff138500a2 100644 --- a/config.c +++ b/config.c @@ -985,19 +985,11 @@ struct parse_event_data { const struct config_parse_options *opts; }; -static int do_event(struct config_source *cs, enum config_event_t type, - struct parse_event_data *data) +static size_t get_corrected_offset(struct config_source *cs, + enum config_event_t type) { - size_t offset; - - if (!data->opts || !data->opts->event_fn) - return 0; - - if (type == CONFIG_EVENT_WHITESPACE && - data->previous_type == type) - return 0; + size_t offset = cs->do_ftell(cs); - offset = cs->do_ftell(cs); /* * At EOF, the parser always "inserts" an extra '\n', therefore * the end offset of the event is the current file position, otherwise @@ -1005,14 +997,44 @@ static int do_event(struct config_source *cs, enum config_event_t type, */ if (type != CONFIG_EVENT_EOF) offset--; + return offset; +} + +static void start_event(struct config_source *cs, enum config_event_t type, + struct parse_event_data *data) +{ + data->previous_type = type; + data->previous_offset = get_corrected_offset(cs, type); +} + +static int flush_event(struct config_source *cs, enum config_event_t type, + struct parse_event_data *data) +{ + if (!data->opts || !data->opts->event_fn) + return 0; + + if (type == CONFIG_EVENT_WHITESPACE && + data->previous_type == type) + return 0; if (data->previous_type != CONFIG_EVENT_EOF && data->opts->event_fn(data->previous_type, data->previous_offset, - offset, cs, data->opts->event_fn_data) < 0) + get_corrected_offset(cs, type), cs, + data->opts->event_fn_data) < 0) return -1; - data->previous_type = type; - data->previous_offset = offset; + return 1; +} + +static int do_event(struct config_source *cs, enum config_event_t type, + struct parse_event_data *data) +{ + int maybe_ret; + + if ((maybe_ret = flush_event(cs, type, data)) < 1) + return maybe_ret; + + start_event(cs, type, data); return 0; }
When handling config-parsing events, the current do_event() handler is a bit confusing; calling it with a specific event type records the initial offset where the event occurred, and runs the supplied callback against the previous event (whose end offset is now known). Split this operation into "start_event" and "flush_event" functions. Then reimplement "do_event" (preserving the original behavior) using the newly split functions. In a later change, we can use these building blocks to also handle "immediate" events, where we want to run the callback without having to calculate an end offset for the event. Signed-off-by: Josh Steadmon <steadmon@google.com> --- config.c | 50 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 14 deletions(-)