Re: [PATCH v14 30/41] Move libified code from builtin/apply.c to apply.{c,h}

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

 



On Sun, Sep 4, 2016 at 1:18 PM, Christian Couder
<christian.couder@xxxxxxxxx> wrote:
> As most of the apply code in builtin/apply.c has been libified by a number of
> previous commits, it can now be moved to apply.{c,h}, so that more code can
> use it.
>
> Helped-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> Helped-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
>  apply.c         | 4731 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  apply.h         |   19 +
>  builtin/apply.c | 4733 +------------------------------------------------------
>  3 files changed, 4751 insertions(+), 4732 deletions(-)

So I wanted to review this patch, so I wrote a patch to review this patch. ;)
https://public-inbox.org/git/82367750-adea-6dee-198a-e39ac7a84b31@xxxxxxxxx/T/#t

> +                       if (!state->apply_in_reverse &&
> +                           state->ws_error_action != nowarn_ws_error)
> +                               check_whitespace(state, line, len, patch->ws_rule);
> +                       added++;
> +                       newlines--;
> +                       trailing = 0;
> +                       break;
> +
> +               /*
> +                * We allow "\ No newline at end of file". Depending
> +                 * on locale settings when the patch was produced we
> +                 * don't know what this line looks like. The only
> +                 * thing we do know is that it begins with "\ ".

The previous three lines are white space broken AFAICT. The seem to be
white space broken in the original location as well, so no need for a reroll
just for this. But in case you do a reroll, you may want to fix these
on the fly?

> +
> +int apply_option_parse_exclude(const struct option *opt,
> +                              const char *arg, int unset)
> +
> +int apply_option_parse_include(const struct option *opt,
> +                              const char *arg, int unset)
> +int apply_option_parse_p(const struct option *opt,
> +                        const char *arg,
> +                        int unset)

These three functions seem slightly different, not just moved.
Oh you removed the static key word!

Apart from the one minor nit and the removal of the static keyword,
the rest is just moved code.

Thanks,
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]