Re: [PATCH v3 4/6] trailer: make args have their own struct

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]