Re: [PATCH v4 08/12] ref-filter: introduce align_atom_parser()

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

 



On Sun, Jan 31, 2016 at 12:42 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> Introduce align_atom_parser() which will parse an 'align' atom and
> store the required alignment position and width in the 'used_atom'
> structure for further usage in populate_value().
>
> Since this patch removes the last usage of match_atom_name(), remove
> the function from ref-filter.c.
>
> Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -55,6 +61,37 @@ static align_type parse_align_position(const char *s)
> +static void align_atom_parser(struct used_atom *atom, const char *arg)
> +{
> +       struct align *align = &atom->u.align;
> +       struct strbuf **s, **to_free;
> +       unsigned int width = ~0U;
> +
> +       if (!arg)
> +               die(_("expected format: %%(align:<width>,<position>)"));
> +       s = to_free = strbuf_split_str_omit_term(arg, ',', 0);
> +
> +       align->position = ALIGN_LEFT;
> +
> +       while (*s) {
> +               int position;
> +               arg = s[0]->buf;

It's confusing to see 'arg' being re-used here for a different
purpose, and leads the reader to wonder if this is done because the
s[0]->buf might be needed outside the loop (when, in fact, it isn't).
It would be better to declare a new variable here in the scope of the
'while' loop to hold this value.

(I might have named the result of the strbuf split 'tokens' or even
short-and-sweet 'v' -- for vector -- and then used 's' for the name of
the new variable here in the 'while' loop, but these name suggestions
aren't particularly important; it is important to declare a new
variable here -- whatever you name it -- rather than re-using 'arg'.)

> +
> +               if (!strtoul_ui(arg, 10, &width))
> +                       ;
> +               else if ((position = parse_align_position(arg)) >= 0)
> +                       align->position = position;
> +               else
> +                       die(_("unrecognized %%(align) argument: %s"), arg);
> +               s++;
> +       }
> +
> +       if (width == ~0U)
> +               die(_("positive width expected with the %%(align) atom"));
> +       align->width = width;
> +       strbuf_list_free(to_free);
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]