[pacman-dev] Some comments on Bryan patches (bugs)

[Nagy Gabor]

> > Nagy Gabor wrote:
> > > Hi!
> > >
> > > I know that I speak too late, but I haven't time/motivation to review
> > > this patch nowadays.
> > >
> > > 1. See alpm/sync.c/sync_prepare(). When user decides to remove
> > > unresolvable targets from the list, our code doesn't remove any targets
> > > from target->packages, just adds the pulled dependencies. The actual
> > > removal is done in the sortbydeps part (sortbydeps is called with list
> > > param!). This works, but it is hard to understand (imho), and it is a
> > > memleak: some pmsyncpkg_t structs magically disappeared.
> > >   
> > 
> > I don't think that it's confusing that it happens this way; the resolve 
> > operation builds up a new list of packages for the transaction, and that 
> > list is then used to replace the trans->packages list.  This is opposed 
> > to before where the resolve operation modified trans->packages in place.
> > 
> > You are right that there is a memory leak there, though; my change 
> > failed to free up any pmsyncpkg_t structs which were in trans->packages 
> > but were not in the replacement list.  I will submit a small and simple 
> > patch which fixes this issue shortly.
> 
> OK, but please insert a short comment somewhere to make our code more
> readable. (The unresolvable targets will be removed later...)

Oh, I was blind, there is a good comment in the code :-)
"/* User wants to remove the unresolvable packages from the
   transaction, so simply drop the unresolvable list.  The
   packages will be removed from the actual transaction when
   the transaction packages are replaced with a
   dependency-reordered list below */"

So simply forget what I said here about comments.

Bye