Re: [PATCH v4 2/5] notes.c: cleanup for "designated init" and "char ptr init"

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

 



Ævar Arnfjörð Bjarmason writes:

> > From: Teng Long <dyroneteng@xxxxxxxxx>
> >
> > Let's do some cleanup for the following two places in "append_edit()".
> >
> > The first place is "char *logmsg;" need to be initialized with NULL.
> > The second place is "struct note_data d = { 0, 0, NULL, STRBUF_INIT };"
> > could be replaced with designated init format.
> >
> > Signed-off-by: Teng Long <dyroneteng@xxxxxxxxx>
> > ---
> >  builtin/notes.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/notes.c b/builtin/notes.c
> > index e57f024824..8ca55ec83e 100644
> > --- a/builtin/notes.c
> > +++ b/builtin/notes.c
> > @@ -566,9 +566,9 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> >  	struct notes_tree *t;
> >  	struct object_id object, new_note;
> >  	const struct object_id *note;
> > -	char *logmsg;
> > +	char *logmsg = NULL;
>
> This change isn't needed, and the compiler will check that we have it
> init'd.
>
> It *is* needed when combined with your 3/5, but even then it's the wrong
> solution. In that change you move the commit_notes() into that "if"
> branch that needs the "logmsg", and it's correct that you need to
> NULL-init it to safely pass it to free().

You are correct, will fix.

> But let's just declare the "logmsg" in that if block then, and move the
> free() over there, that reduces the scope it's in.
>
> >  	const char * const *usage;
> > -	struct note_data d = { 0, 0, NULL, STRBUF_INIT };
> > +	struct note_data d = { .buf = STRBUF_INIT };
>
> This change is good, but then let's change the other such case in this
> file to a designated init too, and make the commit message etc. be about
> using designated init here.

OK.

Thanks.



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

  Powered by Linux