Clean up how game state file sizes are checked

New struct empfile member nent replaces ef_open() parameter nelt.
Cleaner, because the expected size is a property of the file, not of
how it's used.  Also fixes empdump to check file sizes.

Complication: with EFF_CREATE, ef_open() creates an empty file, to be
extended to the correct size.  Callers passed nelt argument -1 along
with EFF_CREATE, to make ef_open() accept the empty file.  Can't do
the same for empfile member nent.  Instead, make ef_open() not check
the (zero) size then.

Replaces commit 5750107b, v4.3.15.

include/file.h

@@ -44,6 +44,7 @@ struct empfile {
     struct castr *cadef;	/* table column selectors (column meta-data) */
     int base;			/* view's base table, else EF_BAD */
     int size;			/* size of a table entry */
+    int nent;			/* #table entries, -1 if variable */
     int flags;			/* only EFF_IMMUTABLE immutable, see below
 				   for use of remaining bits */
 
@@ -209,7 +210,7 @@ extern void ef_mark_fresh(int, void *);
 extern void *ef_ptr(int, int);
 extern char *ef_nameof(int);
 extern time_t ef_mtime(int);
-extern int ef_open(int, int, int);
+extern int ef_open(int, int);
 extern int ef_open_view(int);
 extern int ef_close(int);
 extern int ef_flush(int);

src/lib/common/file.c

@@ -66,12 +66,11 @@ static unsigned ef_generation;
  * Open the file-backed table TYPE (EF_SECTOR, ...).
  * HOW are flags to control operation.  Naturally, immutable flags are
  * not permitted.
- * If NELT is non-negative, the table must have that many elements.
+ * The table must not be already open.
  * Return non-zero on success, zero on failure.
- * You must call ef_close() before the next ef_open().
  */
 int
-ef_open(int type, int how, int nelt)
+ef_open(int type, int how)
 {
     struct empfile *ep;
     int oflags, fd, fsiz, fids, nslots;
@@ -105,9 +104,9 @@ ef_open(int type, int how, int nelt)
 	return 0;
     }
     fids = fsiz / ep->size;
-    if (nelt >= 0 && nelt != fids) {
+    if (ep->nent >= 0 && ep->nent != fids && !(how & EFF_CREATE)) {
 	logerror("Can't open %s (got %d records instead of %d)",
-		 ep->file, fids, nelt);
+		 ep->file, fids, ep->nent);
 	close(fd);
 	return 0;
     }
@@ -117,6 +116,7 @@ ef_open(int type, int how, int nelt)
 	/* ep->cache already points to space for ep->csize elements */
 	if (how & EFF_MEM) {
 	    if (fids > ep->csize) {
+		CANT_HAPPEN(ep->nent >= 0); /* insufficient static cache */
 		logerror("Can't open %s (file larger than %d records)",
 			 ep->file, ep->csize);
 		close(fd);
@@ -239,7 +239,9 @@ ef_open_view(int type)
     if (ef_check(base) < 0)
 	return 0;
     if (CANT_HAPPEN(!(ef_flags(base) & EFF_MEM)
-		    || ep->file || ep->size != empfile[base].size
+		    || ep->file
+		    || ep->size != empfile[base].size
+		    || ep->nent != empfile[base].nent
 		    || ep->cache || ep->oninit || ep->postread
 		    || ep->prewrite || ep->onresize))
 	return -1;

src/lib/common/filetable.c

@@ -60,30 +60,29 @@
 
 /* Initializers for members flags... */
 /* Unmapped cache */
-#define UNMAPPED_CACHE(type, flags) \
+#define UNMAPPED_CACHE(type, nent, flags)	\
-    sizeof(type), (flags), NULL, 0, 0, 0, 0, -1, NULL, NULL, NULL, NULL
+    sizeof(type), (nent), (flags), NULL,	\
+    0, 0, 0, 0, -1, NULL, NULL, NULL, NULL
 /*
  * Mapped cache, array with known size.
  * Members cids, fids are not set.
  */
 #define ARRAY_CACHE(array, flags) \
-    sizeof(*(array)), (flags), (char *)(array), \
+    sizeof(*(array)), -1, (flags), (char *)(array),	\
     SZ((array)), 0, 0, 0, -1, NULL, NULL, NULL, NULL
 /*
  * Mapped cache, array with unknown size.
  * Members csize, cids, fids are not set.
  */
 #define PTR_CACHE(ptr, flags) \
-    sizeof(*(ptr)), (flags), (char *)(ptr), \
+    sizeof(*(ptr)), -1, (flags), (char *)(ptr),	\
     0, 0, 0, 0, -1, NULL, NULL, NULL, NULL
 /*
- * Array-backed table.
+ * Array-backed table of fixed size.
- * The array's last element is the sentinel.
  */
-#define ARRAY_TABLE(array, flags)				\
+#define ARRAY_TABLE(array, nent, flags)				\
-    sizeof(*(array)), (flags), (char *)(array),			\
+    sizeof(*(array)), (nent), (flags), (char *)(array),		\
-    SZ((array)), 0, SZ((array)) - 1, SZ((array)) - 1, -1,	\
+    SZ((array)), 0, (nent), (nent), -1, NULL, NULL, NULL, NULL
-    NULL, NULL, NULL, NULL
 
 /* Common configuration table flags */
 #define EFF_CFG (EFF_PRIVATE | EFF_MEM | EFF_STATIC | EFF_SENTINEL)
@@ -115,46 +114,48 @@ struct empfile empfile[] = {
     /*
      * Dynamic game data
      *
-     * All caches unmapped.  EF_MAP and EF_BMAP get a bogus size here.
+     * All caches unmapped.  EF_SECTOR gets bogus nent here.  EF_MAP
-     * Fixed up by empfile_fixup().
+     * and EF_BMAP get a bogus size here.  Fixed up by
+     * empfile_fixup().
      */
     {EF_SECTOR, "sect", "sector", sect_ca, EF_BAD,
-     UNMAPPED_CACHE(struct sctstr, EFF_TYPED | EFF_XY | EFF_OWNER)},
+     UNMAPPED_CACHE(struct sctstr, -1, EFF_TYPED | EFF_XY | EFF_OWNER)},
     {EF_SHIP, "ship", "ship", ship_ca, EF_BAD,
-     UNMAPPED_CACHE(struct shpstr,
+     UNMAPPED_CACHE(struct shpstr, -1,
 		    EFF_TYPED | EFF_XY | EFF_OWNER | EFF_GROUP)},
     {EF_PLANE, "plane", "plane", plane_ca, EF_BAD,
-     UNMAPPED_CACHE(struct plnstr,
+     UNMAPPED_CACHE(struct plnstr, -1,
 		    EFF_TYPED | EFF_XY | EFF_OWNER | EFF_GROUP)},
     {EF_LAND, "land", "land", land_ca, EF_BAD,
-     UNMAPPED_CACHE(struct lndstr,
+     UNMAPPED_CACHE(struct lndstr, -1,
 		    EFF_TYPED | EFF_XY | EFF_OWNER | EFF_GROUP)},
     {EF_NUKE, "nuke", "nuke", nuke_ca, EF_BAD,
-     UNMAPPED_CACHE(struct nukstr, EFF_TYPED | EFF_XY | EFF_OWNER)},
+     UNMAPPED_CACHE(struct nukstr, -1, EFF_TYPED | EFF_XY | EFF_OWNER)},
     {EF_NEWS, "news", "news", news_ca, EF_BAD,
-     UNMAPPED_CACHE(struct nwsstr, 0)},
+     UNMAPPED_CACHE(struct nwsstr, -1, 0)},
     {EF_TREATY, "treaty", "treaty", treaty_ca, EF_BAD,
-     UNMAPPED_CACHE(struct trtstr, EFF_TYPED)},
+     UNMAPPED_CACHE(struct trtstr, -1, EFF_TYPED)},
     {EF_TRADE, "trade", "trade", trade_ca, EF_BAD,
-     UNMAPPED_CACHE(struct trdstr, EFF_TYPED | EFF_OWNER)},
+     UNMAPPED_CACHE(struct trdstr, -1, EFF_TYPED | EFF_OWNER)},
     {EF_POWER, "pow", "power", NULL, EF_BAD,
-     UNMAPPED_CACHE(struct powstr, 0)},
+     UNMAPPED_CACHE(struct powstr, -1, 0)},
     {EF_NATION, "nat", "nation", nat_ca, EF_BAD,
-     UNMAPPED_CACHE(struct natstr, EFF_TYPED | EFF_OWNER)},
+     UNMAPPED_CACHE(struct natstr, MAXNOC, EFF_TYPED | EFF_OWNER)},
     {EF_LOAN, "loan", "loan", loan_ca, EF_BAD,
-     UNMAPPED_CACHE(struct lonstr, EFF_TYPED)},
+     UNMAPPED_CACHE(struct lonstr, -1, EFF_TYPED)},
     {EF_MAP, "map", "map", NULL, EF_BAD,
-     0, 0, NULL, 0, 0, 0, 0, -1, NULL, NULL, NULL, NULL},
+     0, MAXNOC, 0, NULL, 0, 0, 0, 0, -1, NULL, NULL, NULL, NULL},
     {EF_BMAP, "bmap", "bmap", NULL, EF_BAD,
-     0, 0, NULL, 0, 0, 0, 0, -1, NULL, NULL, NULL, NULL},
+     0, MAXNOC, 0, NULL, 0, 0, 0, 0, -1, NULL, NULL, NULL, NULL},
     {EF_COMM, "commodity", "commodity", commodity_ca, EF_BAD,
-     UNMAPPED_CACHE(struct comstr, EFF_TYPED | EFF_OWNER)},
+     UNMAPPED_CACHE(struct comstr, -1, EFF_TYPED | EFF_OWNER)},
     {EF_LOST, "lost", "lostitems", lost_ca, EF_BAD,
-     UNMAPPED_CACHE(struct loststr, EFF_TYPED | EFF_OWNER)},
+     UNMAPPED_CACHE(struct loststr, -1, EFF_TYPED | EFF_OWNER)},
     {EF_REALM, "realm", "realms", realm_ca, EF_BAD,
-     UNMAPPED_CACHE(struct realmstr, EFF_TYPED | EFF_OWNER)},
+     UNMAPPED_CACHE(struct realmstr, MAXNOC * MAXNOR,
+		    EFF_TYPED | EFF_OWNER)},
     {EF_GAME, "game", "game", game_ca, EF_BAD,
-     UNMAPPED_CACHE(struct gamestr, EFF_TYPED)},
+     UNMAPPED_CACHE(struct gamestr, 1, EFF_TYPED)},
 
     /*
      * Static game data (configuration)
@@ -179,7 +180,7 @@ struct empfile empfile[] = {
     {EF_NUKE_CHR, "nuke-chr", "nuke.config", nchr_ca, EF_BAD,
      ARRAY_CACHE(nchr, EFF_CFG)},
     {EF_NEWS_CHR, "news-chr", NULL, rpt_ca, EF_BAD,
-     ARRAY_TABLE(rpt, EFF_CFG)},
+     ARRAY_TABLE(rpt, N_MAX_VERB + 1, EFF_CFG)},
     {EF_INFRASTRUCTURE, "infrastructure", "infra.config", intrchr_ca, EF_BAD,
      ARRAY_CACHE(intrchr, EFF_CFG)},
     /*
@@ -193,9 +194,9 @@ struct empfile empfile[] = {
      * nsc_init().
      */
     {EF_TABLE, "table", NULL, empfile_ca, EF_BAD,
-     ARRAY_TABLE(empfile, EFF_CFG)},
+     ARRAY_TABLE(empfile, EF_MAX, EFF_CFG)},
     {EF_VERSION, "version", NULL, NULL, EF_BAD,
-     sizeof(PACKAGE_STRING), EFF_STATIC, version, 1, 0, 1, 1, -1,
+     sizeof(PACKAGE_STRING), -1, EFF_STATIC, version, 1, 0, 1, 1, -1,
      NULL, NULL, NULL, NULL},
     {EF_META, "meta", NULL, mdchr_ca, EF_BAD,
      PTR_CACHE(mdchr_ca, EFF_CFG)},
@@ -232,17 +233,17 @@ struct empfile empfile[] = {
 
     /* Views */
     {EF_COUNTRY, "country", NULL, cou_ca, EF_NATION,
-     UNMAPPED_CACHE(struct natstr, EFF_TYPED | EFF_OWNER)},
+     UNMAPPED_CACHE(struct natstr, MAXNOC, EFF_TYPED | EFF_OWNER)},
 
     /* Sentinel */
     {EF_BAD, NULL, NULL, NULL, EF_BAD,
-     0, 0, NULL, 0, 0, 0, 0, -1, NULL, NULL, NULL, NULL},
+     0, -1, 0, NULL, 0, 0, 0, 0, -1, NULL, NULL, NULL, NULL},
 };
 
 static void
 ef_fix_size(struct empfile *ep, int n)
 {
-    ep->cids = ep->fids = n;
+    ep->nent = ep->cids = ep->fids = n;
     ep->csize = n + 1;
 }
 
@@ -270,5 +271,6 @@ empfile_init(void)
 void
 empfile_fixup(void)
 {
+    empfile[EF_SECTOR].nent = WORLD_SZ();
     empfile[EF_MAP].size = empfile[EF_BMAP].size = WORLD_SZ();
 }

src/lib/subs/fileinit.c

@@ -34,8 +34,7 @@
 #include <config.h>
 
 #include "file.h"
-#include "nat.h"
+#include "nsc.h"
-#include "optlist.h"
 #include "prototypes.h"
 #include "unit.h"
 
@@ -92,23 +91,23 @@ ef_open_srv(void)
 {
     int failed = 0;
 
-    failed |= !ef_open(EF_NATION, EFF_MEM, MAXNOC);
+    failed |= !ef_open(EF_NATION, EFF_MEM);
-    failed |= !ef_open(EF_SECTOR, EFF_MEM, WORLD_SZ());
+    failed |= !ef_open(EF_SECTOR, EFF_MEM);
-    failed |= !ef_open(EF_SHIP, EFF_MEM, -1);
+    failed |= !ef_open(EF_SHIP, EFF_MEM);
-    failed |= !ef_open(EF_PLANE, EFF_MEM, -1);
+    failed |= !ef_open(EF_PLANE, EFF_MEM);
-    failed |= !ef_open(EF_LAND, EFF_MEM, -1);
+    failed |= !ef_open(EF_LAND, EFF_MEM);
-    failed |= !ef_open(EF_GAME, EFF_MEM, 1);
+    failed |= !ef_open(EF_GAME, EFF_MEM);
-    failed |= !ef_open(EF_NEWS, 0, -1);
+    failed |= !ef_open(EF_NEWS, 0);
-    failed |= !ef_open(EF_LOAN, 0, -1);
+    failed |= !ef_open(EF_LOAN, 0);
-    failed |= !ef_open(EF_TREATY, 0, -1);
+    failed |= !ef_open(EF_TREATY, 0);
-    failed |= !ef_open(EF_NUKE, EFF_MEM, -1);
+    failed |= !ef_open(EF_NUKE, EFF_MEM);
-    failed |= !ef_open(EF_POWER, 0, -1);
+    failed |= !ef_open(EF_POWER, 0);
-    failed |= !ef_open(EF_TRADE, 0, -1);
+    failed |= !ef_open(EF_TRADE, 0);
-    failed |= !ef_open(EF_MAP, EFF_MEM, MAXNOC);
+    failed |= !ef_open(EF_MAP, EFF_MEM);
-    failed |= !ef_open(EF_BMAP, EFF_MEM, MAXNOC);
+    failed |= !ef_open(EF_BMAP, EFF_MEM);
-    failed |= !ef_open(EF_COMM, 0, -1);
+    failed |= !ef_open(EF_COMM, 0);
-    failed |= !ef_open(EF_LOST, 0, -1);
+    failed |= !ef_open(EF_LOST, 0);
-    failed |= !ef_open(EF_REALM, EFF_MEM, MAXNOC * MAXNOR);
+    failed |= !ef_open(EF_REALM, EFF_MEM);
     if (!failed)
 	failed |= ef_open_view(EF_COUNTRY);
     if (failed) {

src/util/empdump.c

@@ -132,7 +132,7 @@ main(int argc, char *argv[])
 
     for (i = 0; i < EF_MAX; i++) {
 	if (EF_IS_GAME_STATE(i)) {
-	    if (!ef_open(i, EFF_MEM | private, -1))
+	    if (!ef_open(i, EFF_MEM | private))
 		exit(1);
 	} else if (EF_IS_VIEW(i)) {
 	    if (ef_open_view(i) < 0)

src/util/fairland.c

@@ -261,7 +261,7 @@ main(int argc, char *argv[])
 	fprintf(stderr, "Can't chdir to %s (%s)\n", gamedir, strerror(errno));
 	exit(EXIT_FAILURE);
     }
-    if (!ef_open(EF_SECTOR, EFF_MEM | EFF_NOTIME, WORLD_SZ())) {
+    if (!ef_open(EF_SECTOR, EFF_MEM | EFF_NOTIME)) {
 	perror("ef_open");
 	exit(1);
     }

src/util/files.c

@@ -136,7 +136,7 @@ main(int argc, char *argv[])
     for (i = 0; i < EF_MAX; i++) {
 	if (!EF_IS_GAME_STATE(i))
 	    continue;
-	if (!ef_open(i, EFF_CREATE | EFF_NOTIME, -1)) {
+	if (!ef_open(i, EFF_CREATE | EFF_NOTIME)) {
 	    perror("ef_open");
 	    exit(1);
 	}