Browse Source

tdb2: cleanup oob handling.

The tdb_oob() function can fail due to errors, as well as because the length
asked for is greater than the size of the file.  Clean that up:

(1) If probe is true, only fail if there's an error, not if the length is
    too great.
(2) Exit tdb_open() if it tdb_oob() probe fails; this helps cut down
    test time for failtest.
(3) Don't set probe to true in tdb_direct() fail; a minor issue, but it means
    we log failure.
Rusty Russell 14 years ago
parent
commit
77658070a3
3 changed files with 30 additions and 25 deletions
  1. 21 17
      ccan/tdb2/io.c
  2. 3 1
      ccan/tdb2/open.c
  3. 6 7
      ccan/tdb2/transaction.c

+ 21 - 17
ccan/tdb2/io.c

@@ -70,7 +70,9 @@ void tdb_mmap(struct tdb_context *tdb)
 /* check for an out of bounds access - if it is out of bounds then
 /* check for an out of bounds access - if it is out of bounds then
    see if the database has been expanded by someone else and expand
    see if the database has been expanded by someone else and expand
    if necessary
    if necessary
-   note that "len" is the minimum length needed for the db
+   note that "len" is the minimum length needed for the db.
+
+   If probe is true, len being too large isn't a failure.
 */
 */
 static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len,
 static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len,
 			      bool probe)
 			      bool probe)
@@ -84,15 +86,16 @@ static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len,
 	       || tdb_has_expansion_lock(tdb));
 	       || tdb_has_expansion_lock(tdb));
 
 
 	if (len <= tdb->file->map_size)
 	if (len <= tdb->file->map_size)
-		return 0;
+		return TDB_SUCCESS;
 	if (tdb->flags & TDB_INTERNAL) {
 	if (tdb->flags & TDB_INTERNAL) {
-		if (!probe) {
-			tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
-				 "tdb_oob len %lld beyond internal"
-				 " malloc size %lld",
-				 (long long)len,
-				 (long long)tdb->file->map_size);
-		}
+		if (probe)
+			return TDB_SUCCESS;
+
+		tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
+			   "tdb_oob len %lld beyond internal"
+			   " malloc size %lld",
+			   (long long)len,
+			   (long long)tdb->file->map_size);
 		return TDB_ERR_IO;
 		return TDB_ERR_IO;
 	}
 	}
 
 
@@ -111,11 +114,12 @@ static enum TDB_ERROR tdb_oob(struct tdb_context *tdb, tdb_off_t len,
 	tdb_unlock_expand(tdb, F_RDLCK);
 	tdb_unlock_expand(tdb, F_RDLCK);
 
 
 	if (st.st_size < (size_t)len) {
 	if (st.st_size < (size_t)len) {
-		if (!probe) {
-			tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
-				   "tdb_oob len %zu beyond eof at %zu",
-				   (size_t)len, st.st_size);
-		}
+		if (probe)
+			return TDB_SUCCESS;
+
+		tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
+			   "tdb_oob len %zu beyond eof at %zu",
+			   (size_t)len, st.st_size);
 		return TDB_ERR_IO;
 		return TDB_ERR_IO;
 	}
 	}
 
 
@@ -242,7 +246,7 @@ static enum TDB_ERROR tdb_write(struct tdb_context *tdb, tdb_off_t off,
 				  "Write to read-only database");
 				  "Write to read-only database");
 	}
 	}
 
 
-	ecode = tdb->methods->oob(tdb, off + len, 0);
+	ecode = tdb->methods->oob(tdb, off + len, false);
 	if (ecode != TDB_SUCCESS) {
 	if (ecode != TDB_SUCCESS) {
 		return ecode;
 		return ecode;
 	}
 	}
@@ -272,7 +276,7 @@ static enum TDB_ERROR tdb_read(struct tdb_context *tdb, tdb_off_t off,
 {
 {
 	enum TDB_ERROR ecode;
 	enum TDB_ERROR ecode;
 
 
-	ecode = tdb->methods->oob(tdb, off + len, 0);
+	ecode = tdb->methods->oob(tdb, off + len, false);
 	if (ecode != TDB_SUCCESS) {
 	if (ecode != TDB_SUCCESS) {
 		return ecode;
 		return ecode;
 	}
 	}
@@ -563,7 +567,7 @@ static void *tdb_direct(struct tdb_context *tdb, tdb_off_t off, size_t len,
 	if (unlikely(!tdb->file->map_ptr))
 	if (unlikely(!tdb->file->map_ptr))
 		return NULL;
 		return NULL;
 
 
-	ecode = tdb_oob(tdb, off + len, true);
+	ecode = tdb_oob(tdb, off + len, false);
 	if (unlikely(ecode != TDB_SUCCESS))
 	if (unlikely(ecode != TDB_SUCCESS))
 		return TDB_ERR_PTR(ecode);
 		return TDB_ERR_PTR(ecode);
 	return (char *)tdb->file->map_ptr + off;
 	return (char *)tdb->file->map_ptr + off;

+ 3 - 1
ccan/tdb2/open.c

@@ -561,7 +561,9 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags,
 	tdb_unlock_open(tdb, openlock);
 	tdb_unlock_open(tdb, openlock);
 
 
 	/* This make sure we have current map_size and mmap. */
 	/* This make sure we have current map_size and mmap. */
-	tdb->methods->oob(tdb, tdb->file->map_size + 1, true);
+	ecode = tdb->methods->oob(tdb, tdb->file->map_size + 1, true);
+	if (unlikely(ecode != TDB_SUCCESS))
+		goto fail;
 
 
 	/* Now it's fully formed, recover if necessary. */
 	/* Now it's fully formed, recover if necessary. */
 	berr = tdb_needs_recovery(tdb);
 	berr = tdb_needs_recovery(tdb);

+ 6 - 7
ccan/tdb2/transaction.c

@@ -348,15 +348,14 @@ static void transaction_write_existing(struct tdb_context *tdb, tdb_off_t off,
 static enum TDB_ERROR transaction_oob(struct tdb_context *tdb, tdb_off_t len,
 static enum TDB_ERROR transaction_oob(struct tdb_context *tdb, tdb_off_t len,
 				      bool probe)
 				      bool probe)
 {
 {
-	if (len <= tdb->file->map_size) {
+	if (len <= tdb->file->map_size || probe) {
 		return TDB_SUCCESS;
 		return TDB_SUCCESS;
 	}
 	}
-	if (!probe) {
-		tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
-			   "tdb_oob len %lld beyond transaction size %lld",
-			   (long long)len,
-			   (long long)tdb->file->map_size);
-	}
+
+	tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_ERROR,
+		   "tdb_oob len %lld beyond transaction size %lld",
+		   (long long)len,
+		   (long long)tdb->file->map_size);
 	return TDB_ERR_IO;
 	return TDB_ERR_IO;
 }
 }