dpkg: Use iterators instead of exposing packageslump directly

TODO

@@ -45,7 +45,6 @@ TODO
    - Use internerr instead of assert, and print more meaninful messages.
    - Make actionfunction return int, and avoid global exitcode variables.
    - Move ACTION and OBSOLETE, etc to ‘myopt.h’.
-   - Refactor packageslump into generic code.
    - Replace raw write and read calls with safe variants (signals etc).
    - Make deb build version a version instead of doing checks over a string.
    - Use enums for currently hardcoded literals (tdus, dependtry, etc).

src/archives.c

@@ -107,9 +107,8 @@ filesavespackage(struct fileinlist *file,
                  struct pkginfo *pkgtobesaved,
                  struct pkginfo *pkgbeinginstalled)
 {
+  struct filepackages_iterator *iter;
   struct pkginfo *divpkg, *thirdpkg;
-  struct filepackages *packageslump;
-  int i;
   
   debug(dbg_eachfiledetail,"filesavespackage file `%s' package %s",
         file->namenode->name,pkgtobesaved->name);
@@ -139,31 +138,31 @@ filesavespackage(struct fileinlist *file,
   /* Look for a 3rd package which can take over the file (in case
    * it's a directory which is shared by many packages.
    */
-  for (packageslump= file->namenode->packages;
-       packageslump;
-       packageslump= packageslump->more) {
-    for (i=0; i < PERFILEPACKAGESLUMP && packageslump->pkgs[i]; i++) {
-      thirdpkg= packageslump->pkgs[i];
-      debug(dbg_eachfiledetail, "filesavespackage ... also in %s",
-            thirdpkg->name);
-      /* Is this not the package being installed or the one being
-       * checked for disappearance ?
-       */
-      if (thirdpkg == pkgbeinginstalled || thirdpkg == pkgtobesaved) continue;
-      /* If !fileslistvalid then we've already disappeared this one, so
-       * we shouldn't try to make it take over this shared directory.
-       */
-      debug(dbg_eachfiledetail,"filesavespackage ...  is 3rd package");
+  iter = filepackages_iter_new(file->namenode);
+  while ((thirdpkg = filepackages_iter_next(iter))) {
+    debug(dbg_eachfiledetail, "filesavespackage ... also in %s",
+          thirdpkg->name);
+
+    /* Is this not the package being installed or the one being
+     * checked for disappearance? */
+    if (thirdpkg == pkgbeinginstalled || thirdpkg == pkgtobesaved)
+      continue;
 
-      if (!thirdpkg->clientdata->fileslistvalid) {
-        debug(dbg_eachfiledetail, "process_archive ... already disappeared!");
-        continue;
-      }
-      /* We've found a package that can take this file. */
-      debug(dbg_eachfiledetail, "filesavespackage ...  taken -- no save");
-      return false;
+    /* If !fileslistvalid then we've already disappeared this one, so
+     * we shouldn't try to make it take over this shared directory. */
+    debug(dbg_eachfiledetail,"filesavespackage ...  is 3rd package");
+
+    if (!thirdpkg->clientdata->fileslistvalid) {
+      debug(dbg_eachfiledetail, "process_archive ... already disappeared!");
+      continue;
     }
+
+    /* We've found a package that can take this file. */
+    debug(dbg_eachfiledetail, "filesavespackage ...  taken -- no save");
+    return false;
   }
+  filepackages_iter_free(iter);
+
   debug(dbg_eachfiledetail, "filesavespackage ... not taken -- save !");
   return true;
 }
@@ -405,13 +404,12 @@ int tarobject(struct TarInfo *ti) {
   struct conffile *conff;
   struct tarcontext *tc= (struct tarcontext*)ti->UserData;
   bool existingdirectory, keepexisting;
-  int statr, i;
+  int statr;
   ssize_t r;
   struct stat stab, stabtmp;
   char databuf[TARBLKSZ];
   struct fileinlist *nifd, **oldnifd;
   struct pkginfo *divpkg, *otherpkg;
-  struct filepackages *packageslump;
   mode_t am;
 
   ensureobstackinit();
@@ -526,96 +524,100 @@ int tarobject(struct TarInfo *ti) {
 
   keepexisting = false;
   if (!existingdirectory) {
-    for (packageslump= nifd->namenode->packages;
-         packageslump;
-         packageslump= packageslump->more) {
-      for (i=0; i < PERFILEPACKAGESLUMP && packageslump->pkgs[i]; i++) {
-        otherpkg= packageslump->pkgs[i];
-        if (otherpkg == tc->pkg) continue;
-        debug(dbg_eachfile, "tarobject ... found in %s",otherpkg->name);
-        if (nifd->namenode->divert && nifd->namenode->divert->useinstead) {
-          /* Right, so we may be diverting this file.  This makes the conflict
-           * OK iff one of us is the diverting package (we don't need to
-           * check for both being the diverting package, obviously).
-           */
-          divpkg= nifd->namenode->divert->pkg;
-          debug(dbg_eachfile, "tarobject ... diverted, divpkg=%s",
-                divpkg ? divpkg->name : "<none>");
-          if (otherpkg == divpkg || tc->pkg == divpkg) continue;
+    struct filepackages_iterator *iter;
+
+    iter = filepackages_iter_new(nifd->namenode);
+    while ((otherpkg = filepackages_iter_next(iter))) {
+      if (otherpkg == tc->pkg)
+        continue;
+      debug(dbg_eachfile, "tarobject ... found in %s", otherpkg->name);
+
+      if (nifd->namenode->divert && nifd->namenode->divert->useinstead) {
+        /* Right, so we may be diverting this file. This makes the conflict
+         * OK iff one of us is the diverting package (we don't need to
+         * check for both being the diverting package, obviously). */
+        divpkg = nifd->namenode->divert->pkg;
+        debug(dbg_eachfile, "tarobject ... diverted, divpkg=%s",
+              divpkg ? divpkg->name : "<none>");
+        if (otherpkg == divpkg || tc->pkg == divpkg)
+          continue;
+      }
+
+      /* Nope ?  Hmm, file conflict, perhaps.  Check Replaces. */
+      switch (otherpkg->clientdata->replacingfilesandsaid) {
+      case 2:
+        keepexisting = true;
+      case 1:
+        continue;
+      }
+
+      /* Is the package with the conflicting file in the “config files only”
+       * state? If so it must be a config file and we can silenty take it
+       * over. */
+      if (otherpkg->status == stat_configfiles)
+        continue;
+
+      /* Perhaps we're removing a conflicting package? */
+      if (otherpkg->clientdata->istobe == itb_remove)
+        continue;
+
+      /* Is the file an obsolete conffile in the other package
+       * and a conffile in the new package? */
+      if ((nifd->namenode->flags & fnnf_new_conff) &&
+          !statr && S_ISREG(stab.st_mode)) {
+        for (conff = otherpkg->installed.conffiles;
+             conff;
+             conff = conff->next) {
+          if (!conff->obsolete)
+            continue;
+          if (stat(conff->name, &stabtmp))
+            if (errno == ENOENT || errno == ENOTDIR || errno == ELOOP)
+              continue;
+            if (stabtmp.st_dev == stab.st_dev &&
+                stabtmp.st_ino == stab.st_ino)
+              break;
         }
-        /* Nope ?  Hmm, file conflict, perhaps.  Check Replaces. */
-	switch (otherpkg->clientdata->replacingfilesandsaid) {
-	case 2:
-	  keepexisting = true;
-	case 1:
-	  continue;
-	}
-        /* Is the package with the conflicting file in the `config files
-         * only' state ?  If so it must be a config file and we can
-         * silenty take it over.
-         */
-        if (otherpkg->status == stat_configfiles) continue;
-        /* Perhaps we're removing a conflicting package ? */
-        if (otherpkg->clientdata->istobe == itb_remove) continue;
-
-	/* Is the file an obsolete conffile in the other package
-	 * and a conffile in the new package ? */
-	if ((nifd->namenode->flags & fnnf_new_conff) &&
-	    !statr && S_ISREG(stab.st_mode)) {
-	  for (conff= otherpkg->installed.conffiles;
-	       conff;
-	       conff= conff->next) {
-	    if (!conff->obsolete)
-	      continue;
-	    if (stat(conff->name, &stabtmp))
-	      if (errno == ENOENT || errno == ENOTDIR || errno == ELOOP)
-		continue;
-	    if (stabtmp.st_dev == stab.st_dev &&
-		stabtmp.st_ino == stab.st_ino)
-	      break;
-	  }
-	  if (conff) {
-	    debug(dbg_eachfiledetail,"tarobject other's obsolete conffile");
-	    /* processarc.c will have copied its hash already. */
-	    continue;
-	  }
-	}
-
-        if (does_replace(tc->pkg, &tc->pkg->available,
-                         otherpkg, &otherpkg->installed)) {
-          printf(_("Replacing files in old package %s ...\n"),otherpkg->name);
-          otherpkg->clientdata->replacingfilesandsaid= 1;
-        } else if (does_replace(otherpkg, &otherpkg->installed,
-                                tc->pkg, &tc->pkg->available)) {
-	  printf(_("Replaced by files in installed package %s ...\n"),
-		 otherpkg->name);
-          otherpkg->clientdata->replacingfilesandsaid= 2;
-          nifd->namenode->flags &= ~fnnf_new_inarchive;
-	  keepexisting = true;
+        if (conff) {
+          debug(dbg_eachfiledetail, "tarobject other's obsolete conffile");
+          /* processarc.c will have copied its hash already. */
+          continue;
+        }
+      }
+
+      if (does_replace(tc->pkg, &tc->pkg->available,
+                       otherpkg, &otherpkg->installed)) {
+        printf(_("Replacing files in old package %s ...\n"),otherpkg->name);
+        otherpkg->clientdata->replacingfilesandsaid = 1;
+      } else if (does_replace(otherpkg, &otherpkg->installed,
+                              tc->pkg, &tc->pkg->available)) {
+        printf(_("Replaced by files in installed package %s ...\n"),
+               otherpkg->name);
+        otherpkg->clientdata->replacingfilesandsaid = 2;
+        nifd->namenode->flags &= ~fnnf_new_inarchive;
+        keepexisting = true;
+      } else {
+        if (!statr && S_ISDIR(stab.st_mode)) {
+          forcibleerr(fc_overwritedir,
+                      _("trying to overwrite directory '%.250s' "
+                        "in package %.250s %.250s with nondirectory"),
+                      nifd->namenode->name, otherpkg->name,
+                      versiondescribe(&otherpkg->installed.version,
+                                      vdew_nonambig));
         } else {
-          if (!statr && S_ISDIR(stab.st_mode)) {
-            forcibleerr(fc_overwritedir,
-                        _("trying to overwrite directory '%.250s' "
-                          "in package %.250s %.250s with nondirectory"),
+          /* WTA: At this point we are replacing something without a Replaces.
+           * if the new object is a directory and the previous object does not
+           * exist assume it's also a directory and don't complain. */
+          if (!(statr && ti->Type == Directory))
+            forcibleerr(fc_overwrite,
+                        _("trying to overwrite '%.250s', "
+                          "which is also in package %.250s %.250s"),
                         nifd->namenode->name, otherpkg->name,
                         versiondescribe(&otherpkg->installed.version,
                                         vdew_nonambig));
-          } else {
-            /* WTA: At this point we are replacing something without a Replaces.
-	     * if the new object is a directory and the previous object does not
-	     * exist assume it's also a directory and don't complain
-	     */
-            if (! (statr && ti->Type==Directory))
-              forcibleerr(fc_overwrite,
-                          _("trying to overwrite '%.250s', "
-                            "which is also in package %.250s %.250s"),
-                          nifd->namenode->name, otherpkg->name,
-                          versiondescribe(&otherpkg->installed.version,
-                                          vdew_nonambig));
-          }
         }
       }
     }
+    filepackages_iter_free(iter);
   }
 
   if (keepexisting) {

src/filesdb.c

@@ -51,6 +51,62 @@
 #include "filesdb.h"
 #include "main.h"
 
+
+/* filepackages support for tracking packages owning a file. */
+
+#define PERFILEPACKAGESLUMP 10
+
+struct filepackages {
+  struct filepackages *more;
+
+  /* pkgs is a NULL-pointer-terminated list; anything after the first NULL
+   * is garbage. */
+  struct pkginfo *pkgs[PERFILEPACKAGESLUMP];
+};
+
+struct filepackages_iterator {
+  struct filepackages *pkg_lump;
+  int pkg_idx;
+};
+
+struct filepackages_iterator *
+filepackages_iter_new(struct filenamenode *fnn)
+{
+  struct filepackages_iterator *iter;
+
+  iter = m_malloc(sizeof(*iter));
+  iter->pkg_lump  = fnn->packages;
+  iter->pkg_idx = 0;
+
+  return iter;
+}
+
+struct pkginfo *
+filepackages_iter_next(struct filepackages_iterator *iter)
+{
+  struct pkginfo *pkg;
+
+  while (iter->pkg_lump) {
+    pkg = iter->pkg_lump->pkgs[iter->pkg_idx];
+
+    if (iter->pkg_idx < PERFILEPACKAGESLUMP && pkg) {
+      iter->pkg_idx++;
+      return pkg;
+    } else {
+      iter->pkg_lump = iter->pkg_lump->more;
+      iter->pkg_idx = 0;
+    }
+  }
+
+  return NULL;
+}
+
+void
+filepackages_iter_free(struct filepackages_iterator *iter)
+{
+  free(iter);
+}
+
 /*** Generic data structures and routines ***/
 
 static bool allpackagesdone = false;

src/filesdb.h

@@ -120,15 +120,10 @@ struct diversion {
   /* The `contested' halves are in this list for easy cleanup. */
 };
 
-#define PERFILEPACKAGESLUMP 10
-
-struct filepackages {
-  struct filepackages *more;
-  struct pkginfo *pkgs[PERFILEPACKAGESLUMP];
-  /* pkgs is a null-pointer-terminated list; anything after the first null
-   * is garbage
-   */
-};
+struct filepackages_iterator;
+struct filepackages_iterator *filepackages_iter_new(struct filenamenode *fnn);
+struct pkginfo *filepackages_iter_next(struct filepackages_iterator *i);
+void filepackages_iter_free(struct filepackages_iterator *i);
 
 void filesdbinit(void);
 

src/help.c

@@ -492,21 +492,22 @@ hasdirectoryconffiles(struct filenamenode *file, struct pkginfo *pkg)
 bool
 isdirectoryinuse(struct filenamenode *file, struct pkginfo *pkg)
 {
-  struct filepackages *packageslump;
-  int i;
+  struct filepackages_iterator *iter;
+  struct pkginfo *other_pkg;
     
   debug(dbg_veryverbose, "isdirectoryinuse `%s' (except %s)", file->name,
         pkg ? pkg->name : "<none>");
-  for (packageslump= file->packages; packageslump; packageslump= packageslump->more) {
-    debug(dbg_veryverbose, "isdirectoryinuse packageslump %s ...",
-          packageslump->pkgs[0] ? packageslump->pkgs[0]->name : "<none>");
-    for (i=0; i < PERFILEPACKAGESLUMP && packageslump->pkgs[i]; i++) {
-      debug(dbg_veryverbose, "isdirectoryinuse considering [%d] %s ...", i,
-            packageslump->pkgs[i]->name);
-      if (packageslump->pkgs[i] == pkg) continue;
-      return true;
-    }
+
+  iter = filepackages_iter_new(file);
+  while ((other_pkg = filepackages_iter_next(iter))) {
+    debug(dbg_veryverbose, "isdirectoryinuse considering %s ...",
+          other_pkg->name);
+    if (other_pkg == pkg)
+      continue;
+    return true;
   }
+  filepackages_iter_free(iter);
+
   debug(dbg_veryverbose, "isdirectoryinuse no");
   return false;
 }

src/processarc.c

@@ -113,7 +113,6 @@ void process_archive(const char *filename) {
   struct fileinlist *cfile;
   struct reversefilelistiter rlistit;
   struct conffile *searchconff, **iconffileslastp, *newiconff;
-  struct filepackages *packageslump;
   struct dependency *dsearch, *newdeplist, **newdeplistlastp;
   struct dependency *newdep, *dep, *providecheck;
   struct deppossi *psearch, **newpossilastp, *possi, *newpossi, *pdep;
@@ -366,6 +365,8 @@ void process_archive(const char *filename) {
   if (conff) {
     push_cleanup(cu_closefile, ehflag_bombout, NULL, 0, 1, (void *)conff);
     while (fgets(conffilenamebuf,MAXCONFFILENAME-2,conff)) {
+      struct filepackages_iterator *iter;
+
       p= conffilenamebuf + strlen(conffilenamebuf);
       assert(p != conffilenamebuf);
       if (p[-1] != '\n')
@@ -390,29 +391,30 @@ void process_archive(const char *filename) {
        * the file we pick one at random.
        */
       searchconff = NULL;
-      for (packageslump= newconff->namenode->packages;
-           packageslump;
-           packageslump= packageslump->more) {
-        for (i=0; i < PERFILEPACKAGESLUMP && packageslump->pkgs[i]; i++) {
-          otherpkg= packageslump->pkgs[i];
-          debug(dbg_conffdetail,"process_archive conffile `%s' in package %s - conff ?",
-                newconff->namenode->name,otherpkg->name);
-          for (searchconff= otherpkg->installed.conffiles;
-               searchconff && strcmp(newconff->namenode->name,searchconff->name);
-               searchconff= searchconff->next)
-            debug(dbg_conffdetail,
-                  "process_archive conffile `%s' in package %s - conff ? not `%s'",
-                  newconff->namenode->name,otherpkg->name,searchconff->name);
-          if (searchconff) {
-            debug(dbg_conff,"process_archive conffile `%s' package=%s %s hash=%s",
-                  newconff->namenode->name,otherpkg->name,
-                  otherpkg == pkg ? "same" : "different!",
-                  searchconff->hash);
-            if (otherpkg == pkg) goto xit_conff_hashcopy_srch;
-          }
+
+      iter = filepackages_iter_new(newconff->namenode);
+      while ((otherpkg = filepackages_iter_next(iter))) {
+        debug(dbg_conffdetail,
+              "process_archive conffile `%s' in package %s - conff ?",
+              newconff->namenode->name, otherpkg->name);
+        for (searchconff = otherpkg->installed.conffiles;
+             searchconff && strcmp(newconff->namenode->name, searchconff->name);
+             searchconff = searchconff->next)
+          debug(dbg_conffdetail,
+                "process_archive conffile `%s' in package %s - conff ? not `%s'",
+                newconff->namenode->name, otherpkg->name, searchconff->name);
+        if (searchconff) {
+          debug(dbg_conff,
+                "process_archive conffile `%s' package=%s %s hash=%s",
+                newconff->namenode->name, otherpkg->name,
+                otherpkg == pkg ? "same" : "different!",
+                searchconff->hash);
+          if (otherpkg == pkg)
+            break;
         }
       }
-    xit_conff_hashcopy_srch:
+      filepackages_iter_free(iter);
+
       if (searchconff) {
         newconff->namenode->oldhash= searchconff->hash;
 	/* we don't copy `obsolete'; it's not obsolete in the new package */
@@ -1158,6 +1160,8 @@ void process_archive(const char *filename) {
    * have claimed `ownership' of all its files.
    */
   for (cfile= newfileslist; cfile; cfile= cfile->next) {
+    struct filepackages_iterator *iter;
+
     if (!(cfile->namenode->flags & fnnf_elide_other_lists)) continue;
     if (cfile->namenode->divert && cfile->namenode->divert->useinstead) {
       divpkg= cfile->namenode->divert->pkg;
@@ -1176,30 +1180,26 @@ void process_archive(const char *filename) {
       debug(dbg_eachfile, "process_archive looking for overwriting `%s'",
             cfile->namenode->name);
     }
-    for (packageslump= cfile->namenode->packages;
-         packageslump;
-         packageslump= packageslump->more) {
-      for (i=0; i < PERFILEPACKAGESLUMP && packageslump->pkgs[i]; i++) {
-        otherpkg= packageslump->pkgs[i];
-        debug(dbg_eachfiledetail, "process_archive ... found in %s\n",otherpkg->name);
-        /* If !fileslistvalid then it's one of the disappeared packages above
-         * and we don't bother with it here, clearly.
-         */
-        if (otherpkg == pkg || !otherpkg->clientdata->fileslistvalid) continue;
-        if (otherpkg == divpkg) {
-          debug(dbg_eachfiledetail, "process_archive ... diverted, skipping\n");
-          continue;
-        }
-
-        /* Found one.  We delete remove the list entry for this file,
-         * (and any others in the same package) and then mark the package
-         * as requiring a reread.
-         */
-        write_filelist_except(otherpkg, otherpkg->clientdata->files, 1);
-        ensure_package_clientdata(otherpkg);
-        debug(dbg_veryverbose, "process_archive overwrote from %s",otherpkg->name);
+    iter = filepackages_iter_new(cfile->namenode);
+    while ((otherpkg = filepackages_iter_next(iter))) {
+      debug(dbg_eachfiledetail, "process_archive ... found in %s\n", otherpkg->name);
+      /* If !fileslistvalid then it's one of the disappeared packages above
+       * and we don't bother with it here, clearly. */
+      if (otherpkg == pkg || !otherpkg->clientdata->fileslistvalid)
+        continue;
+      if (otherpkg == divpkg) {
+        debug(dbg_eachfiledetail, "process_archive ... diverted, skipping\n");
+        continue;
       }
+
+      /* Found one. We delete remove the list entry for this file,
+       * (and any others in the same package) and then mark the package
+       * as requiring a reread. */
+      write_filelist_except(otherpkg, otherpkg->clientdata->files, 1);
+      ensure_package_clientdata(otherpkg);
+      debug(dbg_veryverbose, "process_archive overwrote from %s", otherpkg->name);
     }
+    filepackages_iter_free(iter);
   }
 
   /* Right, the package we've unpacked is now in a reasonable state.

src/query.c

@@ -192,8 +192,9 @@ listpackages(const char *const *argv)
 }
 
 static int searchoutput(struct filenamenode *namenode) {
-  int found, i;
-  struct filepackages *packageslump;
+  struct filepackages_iterator *iter;
+  struct pkginfo *pkg_owner;
+  int found;
 
   if (namenode->divert) {
     const char *name_from = namenode->divert->camefrom ?
@@ -212,15 +213,16 @@ static int searchoutput(struct filenamenode *namenode) {
     }
   }
   found= 0;
-  for (packageslump= namenode->packages;
-       packageslump;
-       packageslump= packageslump->more) {
-    for (i=0; i < PERFILEPACKAGESLUMP && packageslump->pkgs[i]; i++) {
-      if (found) fputs(", ",stdout);
-      fputs(packageslump->pkgs[i]->name,stdout);
-      found++;
-    }
+
+  iter = filepackages_iter_new(namenode);
+  while ((pkg_owner = filepackages_iter_next(iter))) {
+    if (found)
+      fputs(", ", stdout);
+    fputs(pkg_owner->name, stdout);
+    found++;
   }
+  filepackages_iter_free(iter);
+
   if (found) printf(": %s\n",namenode->name);
   return found + (namenode->divert ? 1 : 0);
 }