[pacman-dev] [PATCH] Fix several memleaks, mostly related to errors handling.

[Xavier]

On Fri, Nov 23, 2007 at 11:51:01PM -0500, Dan McGee wrote:
> This looks good, but take a look at my one thought below in the patch.
> 
> On Nov 23, 2007 6:19 PM, Chantry Xavier <shiningxc at gmail.com> wrote:
> > @@ -614,42 +615,35 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
> >                         if(errorout) {
> > +                               /* The last conflict was unresolvable, so we duplicate it and add it to *data */
> >                                 pm_errno = PM_ERR_CONFLICTING_DEPS;
> > +                               if(data) {
> > +                                       pmconflict_t *lastconflict = conflict;
> > +                                       if((conflict = malloc(sizeof(pmconflict_t))) == NULL) {
> > +                                               _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %zd bytes\n"),
> > +                                                               sizeof(pmconflict_t));
> > +                                               FREELIST(*data);
> > +                                               pm_errno = PM_ERR_MEMORY;
> Can the above be replaced by a call to MALLOC?

That's the first thing I thought about, but I don't know if that is possible.
There are two lists that can only be freed after the error happens : *data
and deps (asked could be freed before).
Or maybe it is possible to do something like :
MALLOC(miss, sizeof(pmdepmissing_t), FREELIST(*data);  FREELIST(deps);
  RET_ERR(PM_ERR_MEMORY, -1));

Though, there is also the "goto cleanup;" that would be skipped, and so the
"alpm_list_free(list);", so that's yet another call to add there.
So that would duplicate the "FREELIST(deps)'" and "alpm_list_free(list);"
calls.

> 
> > +                                       } else {
> > +                                               *conflict = *lastconflict;
> > +                                               *data = alpm_list_add(*data, conflict);
> > +                                       }
> > +                               }
> > +                               FREELIST(asked);
> > +                               FREELIST(deps);
> >                                 ret = -1;
> >                                 goto cleanup;
> >                         }
> > -                       FREELIST(deps);
> >                         FREELIST(asked);
> > +                       FREELIST(deps);
> >                 }
> >                 EVENT(trans, PM_TRANS_EVT_INTERCONFLICTS_DONE, NULL, NULL);
> >         }