On Fri, Oct 14, 2016 at 10:38 AM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> Improve type safety by making arguments (whether from configuration or
> from the command line) have their own "struct arg_item" type, separate
> from the "struct trailer_item" type used to represent the trailers in
> the buffer being manipulated.
>
> This change also prepares "struct trailer_item" to be further
> differentiated from "struct arg_item" in future patches.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
> trailer.c | 135 +++++++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 85 insertions(+), 50 deletions(-)
>
> diff --git a/trailer.c b/trailer.c
> index 54cc930..a9ed3f8 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -29,6 +29,12 @@ struct trailer_item {
> struct list_head list;
> char *token;
> char *value;
> +};
> +
> +struct arg_item {
> + struct list_head list;
> + char *token;
> + char *value;
> struct conf_info conf;
> };
(Unrelated side note:) When first seeing this diff, I assumed the diff
heuristic is going wild, because it doesn't add a full struct.
But on a second closer look, I realize this is the only correct diff,
because we do not account for moved lines from one struct to the
other.
> static void add_arg_to_input_list(struct trailer_item *on_tok,
> - struct trailer_item *arg_tok)
> + struct arg_item *arg_tok)
> {
> - if (after_or_end(arg_tok->conf.where))
> - list_add(&arg_tok->list, &on_tok->list);
> + int aoe = after_or_end(arg_tok->conf.where);
> + struct trailer_item *to_add = trailer_from_arg(arg_tok);
> + if (aoe)
The use of an extra variable here is more readable than my
imagined version of inlining to_add into the list_add calls
just to save aoe.
Looks good to me.
Stefan