On Fri, Aug 05, 2011 at 09:19:43AM -0400, Dan McGee wrote: > On Thu, Aug 4, 2011 at 9:01 AM, Dave Reisner <[email protected]> wrote: > > So it occurs to me that while it's all well and good to have a portable > > package cache cleaner, it's no good at all if it all hinges on a GNU > > sort flag (-V) which poorly approximates what vercmp does. Would we have > > room for sort tool that implements alpm_pkg_vercmp? A simple > > implementation could be as easy as the below... I think this would get a > > lot of mileage out of the AUR/pacman wrapper crowd. > > > > thoughts? > > Looks like this just takes a list of newline-separated versions on > stdin then? Seems pretty reasonable, although argument support would > be good as well. I'm also wondering whether supporting `pacman -Q` > style input would be good as well (with package names and version > numbers); thus it would be a smart sort going first by package name, > but then interpreting the second component as a version rather than > string. > > > #include <alpm.h> > > #include <stdio.h> > > #include <stdlib.h> > > #include <string.h> > > > > #define MAXLEN 4096 > If the tool is new, I'd prefer to avoid any buffer/line length restrictions. > > > > > int vercmp(const void *p1, const void *p2) { > > return alpm_pkg_vercmp(*(const char**)p1, *(const char**)p2); > > } > > > > int main(void) { > coding style, brace on next line. > > char **pkglist = NULL; > > char *pkg; > This is a tad nitpicky, but since we never load actual package > objects, might be wise to call these namelist and name instead. > > char buf[MAXLEN + 1]; > fgets reads at most n-1 characters and always null-terminates; no need > for the extra byte. > > int i, pkgcount = 0; > offsets should use size_t. > > > > while (fgets(buf, MAXLEN + 1, stdin)) { > If you heed my advice above the +1 will have to go away here. > > pkg = strndup(buf, MAXLEN); > You already know buf is well-bounded, correct? strdup() is sufficient. > > *(pkg + strlen(pkg) - 1) = '\0'; > I'm going to throw a CRLF file at this to test. :) > > pkglist = realloc(pkglist, ++pkgcount * sizeof(*pkglist)); > Would be best to not reallocate every iteration, but instead do smart > progression- start with something small/sane like 4, then double it > each realloc. Also, the ++ works, but I'm not a fan of it inline like > this. > > *(pkglist + pkgcount - 1) = pkg; > If pkglist is a * instead of a **, I think you will be able to do something > like > pkglist[pkgcount - 1] = pkg; > instead. That also simplifies the dereferences needed in the printf() > below as you can just do pkglist[i] instead. This is all untested, but > I'm basing it roughly off what I ended up doing for the files array > stuff in be_local.c. > > } > > > > qsort(pkglist, pkgcount, sizeof(char*), vercmp); > char(space)*, style rules. > > > > for (i = 0; i < pkgcount; i++) { > > printf("%s\n", *(pkglist + i)); > > free(*(pkglist + i)); > > } > > if (pkgcount > 0) { > > free(pkglist); > > } > > > > return 0; > > } >
Of course. All lovely suggestions which will be taken into account. This was just a proof of concept to get feedback before I spend any time on it. Since it seems like I'm not crazy for thinking that this is something useful, I'll clean it up to our standards. d
