From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Tue, 23 Feb 2010 22:12:54 GMT
Subject: Re: [PATCH] rerere: fix memory leak if rerere images can't be read
Message-ID: <alpine.DEB.1.00.1002232312090.3980@intel-tinevez-2-302>
In-Reply-To: <36ca99e91002231356u189b80ebka7ae8caf89ab9c10@mail.gmail.com>

Hi,

On Tue, 23 Feb 2010, Bert Wesarg wrote:

> On Tue, Feb 23, 2010 at 22:26, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > On Tue, 23 Feb 2010, Bert Wesarg wrote:
> >
> >> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> >> ---
> >>  rerere.c |    6 ++++--
> >>  1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > Looks good to me, except...
> >
> >> diff --git a/rerere.c b/rerere.c
> >> index d1d3e75..9ca4cb8 100644
> >> --- a/rerere.c
> >> +++ b/rerere.c
> >> @@ -364,16 +364,17 @@ static int find_conflict(struct string_list *conflict)
> >>  static int merge(const char *name, const char *path)
> >>  {
> >>       int ret;
> >> -     mmfile_t cur, base, other;
> >> +     mmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};
> >>       mmbuffer_t result = {NULL, 0};
> >>
> >>       if (handle_file(path, NULL, rerere_path(name, "thisimage")) < 0)
> >>               return 1;
> >>
> >> +     ret = 1;
> >
> > This initialization can come earlier, at declaration time.
> I thought about it, but I think it is clearer to put just in front of
> the condition which may fail.

Well, to _this_ developer, it is clearer when a variable has been 
initialized in any case. No need to think about how it could be used 
uninitialized. But if you insist...

Ciao,
Dscho