[Rpm-maint] [Suse patch] Open all databases before chroot()

[Panu Matilainen]

On Tue, 5 Jun 2007, Michael Schroeder wrote:

> On Mon, Jun 04, 2007 at 04:45:59PM +0300, Panu Matilainen wrote:
>> Whether this is 100% correct fix or not, but it seems to fix real
>> problems. Eg:
>> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=173285
>> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=106057
>> (the related Suse bugs are private unfortunately)
>>
>> AFAICT it doesn't have serious side-effects either, so I'm considering on
>> applying - let me know if you disagree.
>
> Note that Jeff changed rpmdbOpenAll() to don't open the temp databases
> in his version, so he doesn't need an extra rpmdbOpenAllButTemporary
> function.

Yeah, I know. Having thought about it a bit more: if opening 
everything but temporary db's works for the chroot case... then nothing 
can require having those temporary db's opened by rpmdbOpenAll(), 
otherwise the chroot case wouldn't work either, right?

I'm tempted to just go the always filter out temporary db's route to 
avoid different behavior in some corner cases. Attached patch does that, 
and removes a bit of code duplication while at it.

 	- Panu -

diff -r dd2753c27a18 lib/transaction.c
--- a/lib/transaction.c	Thu Jun 07 21:51:59 2007 +0300
+++ b/lib/transaction.c	Fri Jun 08 07:29:25 2007 +0300
@@ -1689,8 +1689,10 @@ rpmMessage(RPMMESS_DEBUG, _("computing %
  	const char * rootDir = rpmtsRootDir(ts);
  	xx = chdir("/");
  	/*@-superuser -noeffect @*/
-	if (rootDir != NULL && strcmp(rootDir, "/") && *rootDir == '/')
+	if (rootDir != NULL && strcmp(rootDir, "/") && *rootDir == '/') {
+	    xx = rpmdbOpenAll(ts->rdb);
  	    xx = chroot(rootDir);
+	}
  	/*@=superuser =noeffect @*/
  	(void) rpmtsSetChrootDone(ts, 1);
      }
diff -r dd2753c27a18 rpmdb/rpmdb.c
--- a/rpmdb/rpmdb.c	Thu Jun 07 21:51:59 2007 +0300
+++ b/rpmdb/rpmdb.c	Fri Jun 08 07:29:44 2007 +0300
@@ -795,6 +795,22 @@ static struct rpmdb_s dbTemplate = {
  };
  /*@=fullinitblock@*/

+static int isTemporaryDB(int rpmtag) 
+{
+    int rc = 0;
+    switch (rpmtag) {
+    case RPMDBI_AVAILABLE:
+    case RPMDBI_ADDED:
+    case RPMDBI_REMOVED:
+    case RPMDBI_DEPENDS:
+	rc = 1;
+	break;
+    default:
+	break;
+    }
+    return rc;
+}
+
  int rpmdbOpenAll(rpmdb db)
  {
      int dbix;
@@ -805,6 +821,9 @@ int rpmdbOpenAll(rpmdb db)
      if (dbiTags != NULL)
      for (dbix = 0; dbix < dbiTagsMax; dbix++) {
  	if (db->_dbi[dbix] != NULL)
+	    continue;
+	/* Filter out temporary databases */
+	if (isTemporaryDB(dbiTags[dbix]))
  	    continue;
  	(void) dbiOpen(db, dbiTags[dbix], db->db_flags);
      }
@@ -1040,16 +1059,8 @@ static int openDatabase(/*@null@*/ const
  	    int rpmtag;

  	    /* Filter out temporary databases */
-	    switch ((rpmtag = dbiTags[dbix])) {
-	    case RPMDBI_AVAILABLE:
-	    case RPMDBI_ADDED:
-	    case RPMDBI_REMOVED:
-	    case RPMDBI_DEPENDS:
+	    if (isTemporaryDB((rpmtag = dbiTags[dbix])))
  		continue;
-		/*@notreached@*/ /*@switchbreak@*/ break;
-	    default:
-		/*@switchbreak@*/ break;
-	    }

  	    dbi = dbiOpen(db, rpmtag, 0);
  	    if (dbi == NULL) {
@@ -2670,14 +2681,11 @@ memset(data, 0, sizeof(*data));
  /*@=boundsread@*/

  	    /*@-branchstate@*/
+	    /* Filter out temporary databases */
+	    if (isTemporaryDB(rpmtag)) 
+		continue;
+
  	    switch (rpmtag) {
-	    /* Filter out temporary databases */
-	    case RPMDBI_AVAILABLE:
-	    case RPMDBI_ADDED:
-	    case RPMDBI_REMOVED:
-	    case RPMDBI_DEPENDS:
-		continue;
-		/*@notreached@*/ /*@switchbreak@*/ break;
  	    case RPMDBI_PACKAGES:
  		dbi = dbiOpen(db, rpmtag, 0);
  		if (dbi == NULL)	/* XXX shouldn't happen */
@@ -3071,14 +3079,11 @@ memset(data, 0, sizeof(*data));
  	    rpmtag = dbiTags[dbix];
  /*@=boundsread@*/

+	    /* Filter out temporary databases */
+	    if (isTemporaryDB(rpmtag)) 
+		continue;
+
  	    switch (rpmtag) {
-	    /* Filter out temporary databases */
-	    case RPMDBI_AVAILABLE:
-	    case RPMDBI_ADDED:
-	    case RPMDBI_REMOVED:
-	    case RPMDBI_DEPENDS:
-		continue;
-		/*@notreached@*/ /*@switchbreak@*/ break;
  	    case RPMDBI_PACKAGES:
  		dbi = dbiOpen(db, rpmtag, 0);
  		if (dbi == NULL)	/* XXX shouldn't happen */
@@ -3660,16 +3665,8 @@ static int rpmdbMoveDatabase(const char
  	    int rpmtag;

  	    /* Filter out temporary databases */
-	    switch ((rpmtag = dbiTags[i])) {
-	    case RPMDBI_AVAILABLE:
-	    case RPMDBI_ADDED:
-	    case RPMDBI_REMOVED:
-	    case RPMDBI_DEPENDS:
+	    if (isTemporaryDB((rpmtag = dbiTags[i])))
  		continue;
-		/*@notreached@*/ /*@switchbreak@*/ break;
-	    default:
-		/*@switchbreak@*/ break;
-	    }

  	    base = tagName(rpmtag);
  	    sprintf(ofilename, "%s/%s/%s", prefix, olddbpath, base);