Christian Couder <christian.couder@xxxxxxxxx> writes:
> After the previous simplifications, it is easy to see that
> there is no need to free the original string passed to
> gitdiff_verify_name(), because this string can be changed
> only when it is NULL.
>
> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> ---
I do not think you need either 1/3 or 2/3 to see that (I actually
think it is easier to see this without 2/3). The caller passes
patch->old_name and
- if that is NULL, then we will either get NULL or a new string
from find_name(); either way, we do not have to worry about
calling free() on the original NULL;
- if that is not NULL, then the only possible value returned from
the function is itself (otherwise it will die()), so we won't be
calling free() in this code.
so I agree with the conclusion, i.e. the conditional free() can go
from these places.
> builtin/apply.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 4cafdaf..9cfa9f4 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -953,21 +953,15 @@ static void gitdiff_verify_name(const char *line, int isnull, char **name, int s
>
> static int gitdiff_oldname(const char *line, struct patch *patch)
> {
> - char *orig = patch->old_name;
> gitdiff_verify_name(line, patch->is_new, &patch->old_name,
> DIFF_OLD_NAME);
> - if (orig != patch->old_name)
> - free(orig);
> return 0;
> }
>
> static int gitdiff_newname(const char *line, struct patch *patch)
> {
> - char *orig = patch->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