Johan Herland <johan@xxxxxxxxxxx> writes:
> Signed-off-by: Johan Herland <johan@xxxxxxxxxxx>
> ---
> builtin/notes.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 1017472..f1480cf 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -399,7 +399,7 @@ static int append_edit(int argc, const char **argv, const char *prefix);
>
> static int add(int argc, const char **argv, const char *prefix)
> {
> - int retval = 0, force = 0;
> + int force = 0;
> const char *object_ref;
> struct notes_tree *t;
> unsigned char object[20], new_note[20];
> @@ -441,6 +441,8 @@ static int add(int argc, const char **argv, const char *prefix)
>
> if (note) {
> if (!force) {
> + free_note_data(&d);
> + free_notes(t);
> if (!d.given) {
It looks a bit strange to refer to d.given after calling a function
that sounds as if it is meant to clear what is recorded in and to
invalidate &d; yes, I can read the implementation to see that
d.given keeps its stale value, but that is something other people
may want to "clean up" later and this reference to d.given will get
in the way when that happens.
At this point of the code, it makes sense to free t in preparation
to either switching to append codepath or erroring out, but does &d
have anything in it already to necessitate its freeing?
> /*
> * Redirect to "edit" subcommand.
> @@ -450,14 +452,11 @@ static int add(int argc, const char **argv, const char *prefix)
> * therefore still in argv[0-1].
> */
> argv[0] = "edit";
> - free_note_data(&d);
> - free_notes(t);
> return append_edit(argc, argv, prefix);
> }
> - retval = error(_("Cannot add notes. Found existing notes "
> + return error(_("Cannot add notes. Found existing notes "
> "for object %s. Use '-f' to overwrite "
> "existing notes"), sha1_to_hex(object));
> - goto out;
> }
> fprintf(stderr, _("Overwriting existing notes for object %s\n"),
> sha1_to_hex(object));
> @@ -474,9 +473,8 @@ static int add(int argc, const char **argv, const char *prefix)
> snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
> is_null_sha1(new_note) ? "removed" : "added", "add");
> commit_notes(t, logmsg);
> -out:
> free_notes(t);
> - return retval;
> + return 0;
> }
>
> static int copy(int argc, const char **argv, const char *prefix)
--
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