Christian Couder <christian.couder@xxxxxxxxx> writes:
> As the value returned by gitdiff_verify_name() is put into the
> same variable that is passed as a parameter to this function,
> it is simpler to pass the address of the variable and have
> gitdiff_verify_name() change the variable itself.
>
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
This change makes the function less useful by restricting the
callers--only the ones that wants in-place update can call it after
this change, while the old function signature allowed a caller to
pass one variable as orig, receive the result in another variable
(and probably compare them).
It does not matter very much for this file scope static helper
either way, and I would probably say the same thing if the patch
were in reverse (i.e. if the patch were loosening the restriction),
but I cannot offhand see why this is an improvement. Puzzled...
> builtin/apply.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 465f954..4cafdaf 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -925,37 +925,37 @@ static int gitdiff_hdrend(const char *line, struct patch *patch)
> #define DIFF_OLD_NAME 0
> #define DIFF_NEW_NAME 1
>
> -static char *gitdiff_verify_name(const char *line, int isnull, char *orig_name, int side)
> +static void gitdiff_verify_name(const char *line, int isnull, char **name, int side)
> {
> - if (!orig_name && !isnull)
> - return find_name(line, NULL, p_value, TERM_TAB);
> + if (!*name && !isnull) {
> + *name = find_name(line, NULL, p_value, TERM_TAB);
> + return;
> + }
>
> - if (orig_name) {
> - int len = strlen(orig_name);
> + if (*name) {
> + int len = strlen(*name);
> char *another;
> if (isnull)
> die(_("git apply: bad git-diff - expected /dev/null, got %s on line %d"),
> - orig_name, linenr);
> + *name, linenr);
> another = find_name(line, NULL, p_value, TERM_TAB);
> - if (!another || memcmp(another, orig_name, len + 1))
> + if (!another || memcmp(another, *name, len + 1))
> die((side == DIFF_NEW_NAME) ?
> _("git apply: bad git-diff - inconsistent new filename on line %d") :
> _("git apply: bad git-diff - inconsistent old filename on line %d"), linenr);
> free(another);
> - return orig_name;
> } else {
> /* expect "/dev/null" */
> if (memcmp("/dev/null", line, 9) || line[9] != '\n')
> die(_("git apply: bad git-diff - expected /dev/null on line %d"), linenr);
> - return NULL;
> }
> }
>
> static int gitdiff_oldname(const char *line, struct patch *patch)
> {
> char *orig = patch->old_name;
> - patch->old_name = gitdiff_verify_name(line, patch->is_new, patch->old_name,
> - DIFF_OLD_NAME);
> + gitdiff_verify_name(line, patch->is_new, &patch->old_name,
> + DIFF_OLD_NAME);
> if (orig != patch->old_name)
> free(orig);
> return 0;
> @@ -964,8 +964,8 @@ static int gitdiff_oldname(const char *line, struct patch *patch)
> static int gitdiff_newname(const char *line, struct patch *patch)
> {
> char *orig = patch->new_name;
> - patch->new_name = gitdiff_verify_name(line, patch->is_delete, patch->new_name,
> - DIFF_NEW_NAME);
> + gitdiff_verify_name(line, patch->is_delete, &patch->new_name,
> + DIFF_NEW_NAME);
> if (orig != patch->new_name)
> free(orig);
> return 0;
--
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