Dan McGee wrote : > On Sun, Mar 29, 2009 at 3:50 PM, changaco <chang...@laposte.net> wrote: > > +*-k \--check*:: > > + Check that the files owned by the package(s) are present on the > > system, > > + that they haven't been deleted. Given no argument it will check all > > the > > + packages installed. > Redundant language, but your native language is not English, so no worries. > > How about: > Check that all files owned by the given packages(s) are present on the > system. If packages are not specified, check all installed packages. > Works for me, changed in the new patch.
> > @@ -510,7 +515,9 @@ static int parseargs(int argc, char *argv[]) > > config->flags |= PM_TRANS_FLAG_DOWNLOADONLY; > > config->flags |= PM_TRANS_FLAG_NOCONFLICTS; > > break; > > - case 'y': (config->op_s_sync)++; break; > > + case 'y': > > + (config->op_s_sync)++; > > + break; > Unnecessary change now, let's just revert these to the original line. > That's right, changed too. > > + if(strncmp(f, "/tmp", 4)==0){ > > + continue; // ignore files in /tmp > > + } > Did you explain why this is necessary? I think all special case > handling should be dropped, and this check is not correct anyway as it > assumes a root of '/'. > I fixed that with the same method you used for the root in your patch. Should I remove the test or not ? > > + FREELIST(damaged); > You get your double free() because you definitely don't want to be > using this macro. What this macro does is free both the list objects > AND their contents. However, when you build the list, you are not > duplicating the list contents, so freeing the list contents here is > something we don't want to do- we only want to free the list itself. > This line should just be a: > > alpm_list_free(damaged); > > > if(filter(pkg)) { > > - display(pkg); > > + if(config->op_q_check){ > > + pkgs = alpm_list_add(pkgs, pkg); > > + } > > + else{ > Add a space before the { and put the "} else {" on one line. > > > + if(config->op_q_check){ > > + check(pkgs); > > + } > > + > > + FREELIST(pkgs); > s/FREELIST/alpm_list_free/ > And this free can really be moved inside the if() statement right > above it, as any adds are protected by the same case. > All fixed according to your patch. > > + display(pkg); > > + } > > } > So with this logic, we end up calling filter() twice for every > package, as you also called it in the check() function above. I > believe that is not necessary, and the filter() call in check should > be dropped. However, that breaks the no explicit packages logic. > > I'm not sure how you want to fix or clean it up, but we shouldn't filter > twice. > > A few random tests: > > dmc...@galway ~/projects/pacman (master) > $ ./src/pacman/pacman -Qtdk > Check complete: 1296 files (693 packages) > > The packages count is definitely wrong there, and looks like it is > wrong in several other filter tests (e.g. pacman -Qtek). > > I'm still not a fan of the flickering progress bar. After you make a > few of these other adjustments I may see what I can come up with. > Problem fixed. See the new patch for details. _______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://www.archlinux.org/mailman/listinfo/pacman-dev