From: Jonathan Tan <jonathantanmy@google.com>
Date: Fri, 04 Aug 2023 21:34:57 GMT
Subject: Re: [RFC PATCH v1.5 3/5] config: report config parse errors using cb
Message-ID: <20230804213457.1174493-1-jonathantanmy@google.com>
In-Reply-To: <20230731234910.94149-4-chooglen@google.com>

Glen Choo <chooglen@google.com> writes:
> +	/*
> +	 * FIXME for whatever reason, do_event passes the _previous_ event, so
> +	 * in order for our callback to receive the error event, we have to call
> +	 * do_event twice
> +	 */
> +	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
> +	do_event(cs, CONFIG_EVENT_ERROR, &event_data);
> +	return -1;
>  }

I think this is because do_event() uses the current position in the
config source as the end_offset and emits the previous event if there is
one (that is, if do_event() has been called before). It can't emit the
current event because it still does not know what the current event's
end_offset is.

I also noticed some more difficult-to-understand things, like _EOF and
_ERROR are never emitted as of current master (i.e. without this patch
set) because they are always the last events to be passed to do_event().
A refactoring to have separate "do" and "flush" would make things
clearer.