* implement mandocdb -t
@ 2011-12-24 16:46 Ingo Schwarze
2011-12-24 23:20 ` Kristaps Dzonsons
0 siblings, 1 reply; 4+ messages in thread
From: Ingo Schwarze @ 2011-12-24 16:46 UTC (permalink / raw)
To: tech
Hi,
finally, here is my implementation of a test mode (-t file ...)
for mandocdb(8). This is the main missing part for pkg_create(8),
so i'd like to get it in quickly.
It involves a few cleanups:
* Bail out on conflicting options.
Distinguish default and config file operation mode.
* Collect some related variables into structs.
Useful because -t calls ofile_argbuild from yet another place.
* Slightly simpler and safer handling of the "of" variable:
Keep the pointer to the tail around, such that stuff could be appended,
and explicitly start at the head whenever starting an iteration.
* Always do all consistency checks.
When any one fails, decide whether to print a message,
or skip the file, or both, or none.
* Do not crash with -a in ofile_dirbuild if there are plain files
in the manual tree root.
This patch is against OpenBSD.
I'm well aware there are a few conflicts, both regarding minor
differences to bsd.lv and with the alignment-neutral database
format, so i'll do a resync afterwards.
Any objections?
Ingo
Index: mandocdb.8
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mandocdb.8,v
retrieving revision 1.13
diff -u -p -r1.13 mandocdb.8
--- mandocdb.8 19 Dec 2011 02:26:33 -0000 1.13
+++ mandocdb.8 24 Dec 2011 16:37:43 -0000
@@ -22,19 +22,22 @@
.Nd index UNIX manuals
.Sh SYNOPSIS
.Nm
-.Op Fl av
+.Op Fl avvv
.Op Fl C Ar file
.Nm
-.Op Fl av
+.Op Fl avvv
.Ar dir ...
.Nm
-.Op Fl v
+.Op Fl vvv
.Fl d Ar dir
.Op Ar
.Nm
-.Op Fl v
+.Op Fl vvv
.Fl u Ar dir
.Op Ar
+.Nm
+.Op Fl vv
+.Fl t Ar
.Sh DESCRIPTION
The
.Nm
@@ -88,6 +91,17 @@ Merge (remove and re-add)
to the database in
.Ar dir
without truncating it.
+.It Fl t Ar
+Check the given
+.Ar files
+for potential problems.
+No databases are modified.
+Implies
+.Fl a
+and one
+.Fl v .
+All diagnostic messages are printed to the standard output;
+the standard error output is not used.
.It Fl u Ar dir
Remove
.Ar
@@ -95,7 +109,19 @@ from the database in
.Ar dir
without truncating it.
.It Fl v
-Display all files added or removed to the index.
+Verbose operation; can be specified multiple times.
+By default,
+.Nm
+operates silently.
+One
+.Fl v
+warns about potential problems.
+A second
+.Fl v
+displays all files added to or removed from the index.
+A third
+.Fl v
+displays additional debugging information.
.El
.Pp
If fatal parse errors are encountered while parsing, the offending file
Index: mandocdb.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mandocdb.c,v
retrieving revision 1.27
diff -u -p -r1.27 mandocdb.c
--- mandocdb.c 20 Dec 2011 00:41:24 -0000 1.27
+++ mandocdb.c 24 Dec 2011 16:37:44 -0000
@@ -41,6 +41,24 @@
#define MANDOC_SRC 0x1
#define MANDOC_FORM 0x2
+/* Access to the mandoc database on disk. */
+
+struct mdb {
+ char idxn[MAXPATHLEN]; /* index db filename */
+ char dbn[MAXPATHLEN]; /* keyword db filename */
+ DB *idx; /* index recno database */
+ DB *db; /* keyword btree database */
+};
+
+/* Stack of temporarily unused index records. */
+
+struct recs {
+ recno_t *stack; /* pointer to a malloc'ed array */
+ size_t size; /* number of allocated slots */
+ size_t cur; /* current number of empty records */
+ recno_t last; /* last record number in the index */
+};
+
/* Tiny list for files. No need to bring in QUEUE. */
struct of {
@@ -64,9 +82,11 @@ struct buf {
/* Operation we're going to perform. */
enum op {
- OP_NEW = 0, /* new database */
+ OP_DEFAULT = 0, /* new dbs from dir list or default config */
+ OP_CONFFILE, /* new databases from custom config file */
OP_UPDATE, /* delete/add entries in existing database */
- OP_DELETE /* delete entries from existing database */
+ OP_DELETE, /* delete entries from existing database */
+ OP_TEST /* change no databases, report potential problems */
};
#define MAN_ARGS DB *hash, \
@@ -89,12 +109,9 @@ static void hash_put(DB *, const stru
static void hash_reset(DB **);
static void index_merge(const struct of *, struct mparse *,
struct buf *, struct buf *, DB *,
- DB *, const char *, DB *, const char *,
- recno_t, const recno_t *, size_t);
-static void index_prune(const struct of *, DB *,
- const char *, DB *, const char *,
- recno_t *, recno_t **, size_t *,
- size_t *);
+ struct mdb *, struct recs *);
+static void index_prune(const struct of *, struct mdb *,
+ struct recs *);
static void ofile_argbuild(int, char *[], struct of **);
static void ofile_dirbuild(const char *, const char *,
const char *, int, struct of **);
@@ -113,7 +130,6 @@ static int pmdoc_Nm(MDOC_ARGS);
static int pmdoc_Sh(MDOC_ARGS);
static int pmdoc_St(MDOC_ARGS);
static int pmdoc_Xr(MDOC_ARGS);
-static void usage(void);
#define MDOCF_CHILD 0x01 /* Automatically index child nodes. */
@@ -257,23 +273,16 @@ mandocdb(int argc, char *argv[])
{
struct mparse *mp; /* parse sequence */
struct manpaths dirs;
+ struct mdb mdb;
+ struct recs recs;
enum op op; /* current operation */
const char *dir;
- char *conf_file;
char *cp;
- char pbuf[PATH_MAX],
- ibuf[MAXPATHLEN], /* index fname */
- fbuf[MAXPATHLEN]; /* btree fname */
+ char pbuf[PATH_MAX];
int ch, i, flags;
- DB *idx, /* index database */
- *db, /* keyword database */
- *hash; /* temporary keyword hashtable */
+ DB *hash; /* temporary keyword hashtable */
BTREEINFO info; /* btree configuration */
- recno_t maxrec; /* last record number in the index */
- recno_t *recs; /* the numbers of all empty records */
- size_t sz1, sz2,
- recsz, /* number of allocated slots in recs */
- reccur; /* current number of empty records */
+ size_t sz1, sz2;
struct buf buf, /* keyword buffer */
dbuf; /* description buffer */
struct of *of; /* list of files for processing */
@@ -287,33 +296,53 @@ mandocdb(int argc, char *argv[])
++progname;
memset(&dirs, 0, sizeof(struct manpaths));
+ memset(&mdb, 0, sizeof(struct mdb));
+ memset(&recs, 0, sizeof(struct recs));
verb = 0;
use_all = 0;
of = NULL;
- db = idx = NULL;
mp = NULL;
hash = NULL;
- recs = NULL;
- recsz = reccur = 0;
- maxrec = 0;
- op = OP_NEW;
+ op = OP_DEFAULT;
dir = NULL;
- conf_file = NULL;
- while (-1 != (ch = getopt(argc, argv, "aC:d:u:v")))
+ while (-1 != (ch = getopt(argc, argv, "aC:d:tu:v")))
switch (ch) {
case ('a'):
use_all = 1;
break;
case ('C'):
- conf_file = optarg;
+ if (op) {
+ fprintf(stderr, "conflicting options\n");
+ goto usage;
+ }
+ dir = optarg;
+ op = OP_CONFFILE;
break;
case ('d'):
+ if (op) {
+ fprintf(stderr, "conflicting options\n");
+ goto usage;
+ }
dir = optarg;
op = OP_UPDATE;
break;
+ case ('t'):
+ dup2(STDOUT_FILENO, STDERR_FILENO);
+ if (op) {
+ fprintf(stderr, "conflicting options\n");
+ goto usage;
+ }
+ op = OP_TEST;
+ use_all = 1;
+ verb++;
+ break;
case ('u'):
+ if (op) {
+ fprintf(stderr, "conflicting options\n");
+ goto usage;
+ }
dir = optarg;
op = OP_DELETE;
break;
@@ -321,13 +350,17 @@ mandocdb(int argc, char *argv[])
verb++;
break;
default:
- usage();
- return((int)MANDOCLEVEL_BADARG);
+ goto usage;
}
argc -= optind;
argv += optind;
+ if (OP_CONFFILE == op && argc > 0) {
+ fprintf(stderr, "too many arguments for -C\n");
+ goto usage;
+ }
+
memset(&info, 0, sizeof(BTREEINFO));
info.lorder = 4321;
info.flags = R_DUP;
@@ -342,32 +375,40 @@ mandocdb(int argc, char *argv[])
buf.cp = mandoc_malloc(buf.size);
dbuf.cp = mandoc_malloc(dbuf.size);
- flags = OP_NEW == op ? O_CREAT|O_TRUNC|O_RDWR : O_CREAT|O_RDWR;
+ flags = O_CREAT | O_RDWR;
+ if (OP_DEFAULT == op || OP_CONFFILE == op)
+ flags |= O_TRUNC;
- if (OP_UPDATE == op || OP_DELETE == op) {
- ibuf[0] = fbuf[0] = '\0';
+ if (OP_TEST == op) {
+ ofile_argbuild(argc, argv, &of);
+ if (NULL == of)
+ goto out;
+ index_merge(of, mp, &dbuf, &buf, hash, &mdb, &recs);
+ goto out;
+ }
- strlcat(fbuf, dir, MAXPATHLEN);
- strlcat(fbuf, "/", MAXPATHLEN);
- sz1 = strlcat(fbuf, MANDOC_DB, MAXPATHLEN);
-
- strlcat(ibuf, dir, MAXPATHLEN);
- strlcat(ibuf, "/", MAXPATHLEN);
- sz2 = strlcat(ibuf, MANDOC_IDX, MAXPATHLEN);
+ if (OP_UPDATE == op || OP_DELETE == op) {
+ strlcat(mdb.dbn, dir, MAXPATHLEN);
+ strlcat(mdb.dbn, "/", MAXPATHLEN);
+ sz1 = strlcat(mdb.dbn, MANDOC_DB, MAXPATHLEN);
+
+ strlcat(mdb.idxn, dir, MAXPATHLEN);
+ strlcat(mdb.idxn, "/", MAXPATHLEN);
+ sz2 = strlcat(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
if (sz1 >= MAXPATHLEN || sz2 >= MAXPATHLEN) {
- fprintf(stderr, "%s: Path too long\n", dir);
+ fprintf(stderr, "%s: path too long\n", dir);
exit((int)MANDOCLEVEL_BADARG);
}
- db = dbopen(fbuf, flags, 0644, DB_BTREE, &info);
- idx = dbopen(ibuf, flags, 0644, DB_RECNO, NULL);
+ mdb.db = dbopen(mdb.dbn, flags, 0644, DB_BTREE, &info);
+ mdb.idx = dbopen(mdb.idxn, flags, 0644, DB_RECNO, NULL);
- if (NULL == db) {
- perror(fbuf);
+ if (NULL == mdb.db) {
+ perror(mdb.dbn);
exit((int)MANDOCLEVEL_SYSERR);
- } else if (NULL == idx) {
- perror(ibuf);
+ } else if (NULL == mdb.idx) {
+ perror(mdb.idxn);
exit((int)MANDOCLEVEL_SYSERR);
}
@@ -376,10 +417,7 @@ mandocdb(int argc, char *argv[])
if (NULL == of)
goto out;
- of = of->first;
-
- index_prune(of, db, fbuf, idx, ibuf,
- &maxrec, &recs, &recsz, &reccur);
+ index_prune(of, &mdb, &recs);
/*
* Go to the root of the respective manual tree.
@@ -393,8 +431,7 @@ mandocdb(int argc, char *argv[])
exit((int)MANDOCLEVEL_SYSERR);
}
index_merge(of, mp, &dbuf, &buf, hash,
- db, fbuf, idx, ibuf,
- maxrec, recs, reccur);
+ &mdb, &recs);
}
goto out;
@@ -417,44 +454,44 @@ mandocdb(int argc, char *argv[])
dirs.paths[i] = mandoc_strdup(cp);
}
} else
- manpath_parse(&dirs, conf_file, NULL, NULL);
+ manpath_parse(&dirs, dir, NULL, NULL);
for (i = 0; i < dirs.sz; i++) {
- ibuf[0] = fbuf[0] = '\0';
+ mdb.idxn[0] = mdb.dbn[0] = '\0';
- strlcat(fbuf, dirs.paths[i], MAXPATHLEN);
- strlcat(fbuf, "/", MAXPATHLEN);
- sz1 = strlcat(fbuf, MANDOC_DB, MAXPATHLEN);
-
- strlcat(ibuf, dirs.paths[i], MAXPATHLEN);
- strlcat(ibuf, "/", MAXPATHLEN);
- sz2 = strlcat(ibuf, MANDOC_IDX, MAXPATHLEN);
+ strlcat(mdb.dbn, dirs.paths[i], MAXPATHLEN);
+ strlcat(mdb.dbn, "/", MAXPATHLEN);
+ sz1 = strlcat(mdb.dbn, MANDOC_DB, MAXPATHLEN);
+
+ strlcat(mdb.idxn, dirs.paths[i], MAXPATHLEN);
+ strlcat(mdb.idxn, "/", MAXPATHLEN);
+ sz2 = strlcat(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
if (sz1 >= MAXPATHLEN || sz2 >= MAXPATHLEN) {
- fprintf(stderr, "%s: Path too long\n",
+ fprintf(stderr, "%s: path too long\n",
dirs.paths[i]);
exit((int)MANDOCLEVEL_BADARG);
}
- if (db)
- (*db->close)(db);
- if (idx)
- (*idx->close)(idx);
+ if (mdb.db)
+ (*mdb.db->close)(mdb.db);
+ if (mdb.idx)
+ (*mdb.idx->close)(mdb.idx);
- db = dbopen(fbuf, flags, 0644, DB_BTREE, &info);
- idx = dbopen(ibuf, flags, 0644, DB_RECNO, NULL);
+ mdb.db = dbopen(mdb.dbn, flags, 0644, DB_BTREE, &info);
+ mdb.idx = dbopen(mdb.idxn, flags, 0644, DB_RECNO, NULL);
- if (NULL == db) {
- perror(fbuf);
+ if (NULL == mdb.db) {
+ perror(mdb.dbn);
exit((int)MANDOCLEVEL_SYSERR);
- } else if (NULL == idx) {
- perror(ibuf);
+ } else if (NULL == mdb.idx) {
+ perror(mdb.idxn);
exit((int)MANDOCLEVEL_SYSERR);
}
if (verb > 2) {
- printf("%s: Truncated\n", fbuf);
- printf("%s: Truncated\n", ibuf);
+ printf("%s: truncated\n", mdb.dbn);
+ printf("%s: truncated\n", mdb.idxn);
}
ofile_free(of);
@@ -463,17 +500,14 @@ mandocdb(int argc, char *argv[])
if (-1 == chdir(dirs.paths[i])) {
perror(dirs.paths[i]);
exit((int)MANDOCLEVEL_SYSERR);
- }
-
- ofile_dirbuild(".", NULL, NULL, 0, &of);
+ }
+ ofile_dirbuild(".", "", "", 0, &of);
if (NULL == of)
continue;
- of = of->first;
-
/*
- * Go to the root of the respective manual tree.
+ * Go to the root of the respective manual tree.
* This must work or no manuals may be found (they're
* indexed relative to the root).
*/
@@ -483,15 +517,14 @@ mandocdb(int argc, char *argv[])
exit((int)MANDOCLEVEL_SYSERR);
}
- index_merge(of, mp, &dbuf, &buf, hash, db, fbuf,
- idx, ibuf, maxrec, recs, reccur);
+ index_merge(of, mp, &dbuf, &buf, hash, &mdb, &recs);
}
out:
- if (db)
- (*db->close)(db);
- if (idx)
- (*idx->close)(idx);
+ if (mdb.db)
+ (*mdb.db->close)(mdb.db);
+ if (mdb.idx)
+ (*mdb.idx->close)(mdb.idx);
if (hash)
(*hash->close)(hash);
if (mp)
@@ -501,30 +534,39 @@ out:
ofile_free(of);
free(buf.cp);
free(dbuf.cp);
- free(recs);
+ free(recs.stack);
return(MANDOCLEVEL_OK);
+
+usage:
+ fprintf(stderr,
+ "usage: %s [-avvv] [-C file] | dir ... | -t file ...\n"
+ " -d dir [file ...] | "
+ "-u dir [file ...]\n",
+ progname);
+
+ return((int)MANDOCLEVEL_BADARG);
}
void
index_merge(const struct of *of, struct mparse *mp,
struct buf *dbuf, struct buf *buf, DB *hash,
- DB *db, const char *dbf, DB *idx, const char *idxf,
- recno_t maxrec, const recno_t *recs, size_t reccur)
+ struct mdb *mdb, struct recs *recs)
{
recno_t rec;
- int ch;
+ int ch, skip;
DBT key, val;
struct mdoc *mdoc;
struct man *man;
- const char *fn, *msec, *mtitle, *arch;
+ const char *fn, *msec, *march, *mtitle;
uint64_t mask;
size_t sv;
unsigned seq;
struct db_val vbuf;
char type;
- for (rec = 0; of; of = of->next) {
+ rec = 0;
+ for (of = of->first; of; of = of->next) {
fn = of->fname;
/*
@@ -544,15 +586,17 @@ index_merge(const struct of *of, struct
if (NULL != mdoc) {
msec = mdoc_meta(mdoc)->msec;
- arch = mdoc_meta(mdoc)->arch;
+ march = mdoc_meta(mdoc)->arch;
+ if (NULL == march)
+ march = "";
mtitle = mdoc_meta(mdoc)->title;
} else if (NULL != man) {
msec = man_meta(man)->msec;
- arch = NULL;
+ march = "";
mtitle = man_meta(man)->title;
} else {
msec = of->sec;
- arch = of->arch;
+ march = of->arch;
mtitle = of->title;
}
@@ -562,24 +606,30 @@ index_merge(const struct of *of, struct
* with the directory where the file is located.
*/
- if (0 == use_all) {
- assert(of->sec);
- assert(msec);
- if (strcasecmp(msec, of->sec))
- continue;
-
- if (NULL == arch) {
- if (NULL != of->arch)
- continue;
- } else if (NULL == of->arch ||
- strcasecmp(arch, of->arch))
- continue;
+ skip = 0;
+ assert(of->sec);
+ assert(msec);
+ if (strcasecmp(msec, of->sec)) {
+ if (verb)
+ fprintf(stderr, "%s: "
+ "section \"%s\" manual "
+ "in \"%s\" directory\n",
+ fn, msec, of->sec);
+ skip = 1;
+ }
+
+ assert(of->arch);
+ assert(march);
+ if (strcasecmp(march, of->arch)) {
+ if (verb)
+ fprintf(stderr, "%s: "
+ "architecture \"%s\" manual "
+ "in \"%s\" directory\n",
+ fn, march, of->arch);
+ skip = 1;
}
- if (NULL == arch)
- arch = "";
-
- /*
+ /*
* By default, skip a file if the title given
* in the file disagrees with the file name.
* If both agree, use the file name as the title,
@@ -588,13 +638,20 @@ index_merge(const struct of *of, struct
assert(of->title);
assert(mtitle);
-
- if (0 == strcasecmp(mtitle, of->title))
+ if (strcasecmp(mtitle, of->title)) {
+ if (verb)
+ fprintf(stderr, "%s: "
+ "title \"%s\" in file "
+ "but \"%s\" in filename\n",
+ fn, mtitle, of->title);
+ skip = 1;
+ } else
mtitle = of->title;
- else if (0 == use_all)
+
+ if (skip && !use_all)
continue;
- /*
+ /*
* The index record value consists of a nil-terminated
* filename, a nil-terminated manual section, and a
* nil-terminated description. Since the description
@@ -608,7 +665,7 @@ index_merge(const struct of *of, struct
buf_appendb(dbuf, fn, strlen(fn) + 1);
buf_appendb(dbuf, msec, strlen(msec) + 1);
buf_appendb(dbuf, mtitle, strlen(mtitle) + 1);
- buf_appendb(dbuf, arch, strlen(arch) + 1);
+ buf_appendb(dbuf, march, strlen(march) + 1);
sv = dbuf->len;
@@ -626,17 +683,22 @@ index_merge(const struct of *of, struct
else
pformatted(hash, buf, dbuf, of);
+ /* Test mode, do not access any database. */
+
+ if (NULL == mdb->db || NULL == mdb->idx)
+ continue;
+
/*
* Reclaim an empty index record, if available.
* Use its record number for all new btree nodes.
*/
- if (reccur > 0) {
- --reccur;
- rec = recs[(int)reccur];
- } else if (maxrec > 0) {
- rec = maxrec;
- maxrec = 0;
+ if (recs->cur > 0) {
+ recs->cur--;
+ rec = recs->stack[(int)recs->cur];
+ } else if (recs->last > 0) {
+ rec = recs->last;
+ recs->last = 0;
} else
rec++;
vbuf.rec = htobe32(rec);
@@ -654,13 +716,13 @@ index_merge(const struct of *of, struct
vbuf.mask = htobe64(mask);
val.size = sizeof(struct db_val);
val.data = &vbuf;
- dbt_put(db, dbf, &key, &val);
+ dbt_put(mdb->db, mdb->dbn, &key, &val);
}
if (ch < 0) {
perror("hash");
exit((int)MANDOCLEVEL_SYSERR);
}
-
+
/*
* Apply to the index. If we haven't had a description
* set, put an empty one in now.
@@ -675,10 +737,10 @@ index_merge(const struct of *of, struct
val.data = dbuf->cp;
val.size = dbuf->len;
- if (verb)
- printf("%s: Added index\n", fn);
+ if (verb > 1)
+ printf("%s: adding to index\n", fn);
- dbt_put(idx, idxf, &key, &val);
+ dbt_put(mdb->idx, mdb->idxn, &key, &val);
}
}
@@ -689,9 +751,7 @@ index_merge(const struct of *of, struct
* in `idx' (zeroing its value size).
*/
static void
-index_prune(const struct of *ofile, DB *db, const char *dbf,
- DB *idx, const char *idxf, recno_t *maxrec,
- recno_t **recs, size_t *recsz, size_t *reccur)
+index_prune(const struct of *ofile, struct mdb *mdb, struct recs *recs)
{
const struct of *of;
const char *fn;
@@ -700,12 +760,12 @@ index_prune(const struct of *ofile, DB *
DBT key, val;
int ch;
- *reccur = 0;
+ recs->cur = 0;
seq = R_FIRST;
- while (0 == (ch = (*idx->seq)(idx, &key, &val, seq))) {
+ while (0 == (ch = (*mdb->idx->seq)(mdb->idx, &key, &val, seq))) {
seq = R_NEXT;
assert(sizeof(recno_t) == key.size);
- memcpy(maxrec, key.data, key.size);
+ memcpy(&recs->last, key.data, key.size);
/* Deleted records are zero-sized. Skip them. */
@@ -723,12 +783,12 @@ index_prune(const struct of *ofile, DB *
if (NULL == memchr(fn, '\0', val.size - 1))
break;
- /*
+ /*
* Search for the file in those we care about.
* XXX: build this into a tree. Too slow.
*/
- for (of = ofile; of; of = of->next)
+ for (of = ofile->first; of; of = of->next)
if (0 == strcmp(fn, of->fname))
break;
@@ -741,55 +801,58 @@ index_prune(const struct of *ofile, DB *
*/
sseq = R_FIRST;
- while (0 == (ch = (*db->seq)(db, &key, &val, sseq))) {
+ while (0 == (ch = (*mdb->db->seq)(mdb->db,
+ &key, &val, sseq))) {
sseq = R_NEXT;
if (sizeof(struct db_val) != val.size)
break;
vbuf = val.data;
- if (*maxrec != betoh32(vbuf->rec))
+ if (recs->last != betoh32(vbuf->rec))
continue;
- if ((ch = (*db->del)(db, &key, R_CURSOR)) < 0)
+ if ((ch = (*mdb->db->del)(mdb->db,
+ &key, R_CURSOR)) < 0)
break;
}
if (ch < 0) {
- perror(dbf);
+ perror(mdb->dbn);
exit((int)MANDOCLEVEL_SYSERR);
} else if (1 != ch) {
- fprintf(stderr, "%s: Corrupt database\n", dbf);
+ fprintf(stderr, "%s: corrupt database\n",
+ mdb->dbn);
exit((int)MANDOCLEVEL_SYSERR);
}
- if (verb)
- printf("%s: Deleted index\n", fn);
+ if (verb > 1)
+ printf("%s: deleting from index\n", fn);
val.size = 0;
- ch = (*idx->put)(idx, &key, &val, R_CURSOR);
+ ch = (*mdb->idx->put)(mdb->idx, &key, &val, R_CURSOR);
if (ch < 0)
break;
cont:
- if (*reccur >= *recsz) {
- *recsz += MANDOC_SLOP;
- *recs = mandoc_realloc
- (*recs, *recsz * sizeof(recno_t));
+ if (recs->cur >= recs->size) {
+ recs->size += MANDOC_SLOP;
+ recs->stack = mandoc_realloc(recs->stack,
+ recs->size * sizeof(recno_t));
}
- (*recs)[(int)*reccur] = *maxrec;
- (*reccur)++;
+ recs->stack[(int)recs->cur] = recs->last;
+ recs->cur++;
}
if (ch < 0) {
- perror(idxf);
+ perror(mdb->idxn);
exit((int)MANDOCLEVEL_SYSERR);
} else if (1 != ch) {
- fprintf(stderr, "%s: Corrupt index\n", idxf);
+ fprintf(stderr, "%s: corrupt index\n", mdb->idxn);
exit((int)MANDOCLEVEL_SYSERR);
}
- (*maxrec)++;
+ recs->last++;
}
/*
@@ -1281,7 +1344,8 @@ pformatted(DB *hash, struct buf *buf, st
size_t len, plen;
if (NULL == (stream = fopen(of->fname, "r"))) {
- perror(of->fname);
+ if (verb)
+ perror(of->fname);
return;
}
@@ -1320,6 +1384,9 @@ pformatted(DB *hash, struct buf *buf, st
line = fgetln(stream, &len);
if (NULL == line || ' ' != *line || '\n' != line[(int)len - 1]) {
+ if (verb)
+ fprintf(stderr, "%s: cannot find NAME section\n",
+ of->fname);
buf_appendb(dbuf, buf->cp, buf->size);
hash_put(hash, buf, TYPE_Nd);
fclose(stream);
@@ -1337,8 +1404,12 @@ pformatted(DB *hash, struct buf *buf, st
if (NULL != (p = strstr(line, "- "))) {
for (p += 2; ' ' == *p || '\b' == *p; p++)
/* Skip to next word. */ ;
- } else
+ } else {
+ if (verb)
+ fprintf(stderr, "%s: no dash in title line\n",
+ of->fname);
p = line;
+ }
if ((plen = strlen(p)) > 70) {
plen = 70;
@@ -1382,14 +1453,14 @@ ofile_argbuild(int argc, char *argv[], s
*/
if (strlcpy(buf, argv[i], sizeof(buf)) >= sizeof(buf)) {
- fprintf(stderr, "%s: Path too long\n", argv[i]);
+ fprintf(stderr, "%s: path too long\n", argv[i]);
continue;
}
- sec = arch = title = NULL;
+ sec = arch = title = "";
src_form = 0;
p = strrchr(buf, '\0');
while (p-- > buf) {
- if (NULL == sec && '.' == *p) {
+ if ('\0' == *sec && '.' == *p) {
sec = p + 1;
*p = '\0';
if ('0' == *sec)
@@ -1400,7 +1471,7 @@ ofile_argbuild(int argc, char *argv[], s
}
if ('/' != *p)
continue;
- if (NULL == title) {
+ if ('\0' == *title) {
title = p + 1;
*p = '\0';
continue;
@@ -1413,8 +1484,14 @@ ofile_argbuild(int argc, char *argv[], s
arch = p + 1;
break;
}
- if (NULL == title)
+ if ('\0' == *title) {
+ if (verb)
+ fprintf(stderr,
+ "%s: cannot deduce title "
+ "from filename\n",
+ argv[i]);
title = buf;
+ }
/*
* Build the file structure.
@@ -1422,10 +1499,8 @@ ofile_argbuild(int argc, char *argv[], s
nof = mandoc_calloc(1, sizeof(struct of));
nof->fname = mandoc_strdup(argv[i]);
- if (NULL != sec)
- nof->sec = mandoc_strdup(sec);
- if (NULL != arch)
- nof->arch = mandoc_strdup(arch);
+ nof->sec = mandoc_strdup(sec);
+ nof->arch = mandoc_strdup(arch);
nof->title = mandoc_strdup(title);
nof->src_form = src_form;
@@ -1433,8 +1508,8 @@ ofile_argbuild(int argc, char *argv[], s
* Add the structure to the list.
*/
- if (verb > 2)
- printf("%s: Scheduling\n", argv[i]);
+ if (verb > 2)
+ printf("%s: scheduling\n", argv[i]);
if (NULL == *of) {
*of = nof;
(*of)->first = nof;
@@ -1468,8 +1543,9 @@ ofile_dirbuild(const char *dir, const ch
int src_form;
if (NULL == (d = opendir(dir))) {
- perror(dir);
- return(0);
+ if (verb)
+ perror(dir);
+ return;
}
while (NULL != (dp = readdir(d))) {
@@ -1490,22 +1566,35 @@ ofile_dirbuild(const char *dir, const ch
* cat<section>/[<arch>/]
*/
- if (NULL == sec) {
+ if ('\0' == *sec) {
if(0 == strncmp("man", fn, 3)) {
src_form |= MANDOC_SRC;
sec = fn + 3;
} else if (0 == strncmp("cat", fn, 3)) {
src_form |= MANDOC_FORM;
sec = fn + 3;
- } else if (use_all)
- sec = fn;
- else
- continue;
- } else if (NULL == arch && (use_all ||
- NULL == strchr(fn, '.')))
+ } else {
+ if (verb) fprintf(stderr, "%s/%s: "
+ "bad section\n", dir, fn);
+ if (use_all)
+ sec = fn;
+ else
+ continue;
+ }
+ } else if ('\0' == *arch) {
+ if (NULL != strchr(fn, '.')) {
+ if (verb) fprintf(stderr, "%s/%s: "
+ "bad architecture\n", dir, fn);
+ if (0 == use_all)
+ continue;
+ }
arch = fn;
- else if (0 == use_all)
- continue;
+ } else {
+ if (verb) fprintf(stderr, "%s/%s: "
+ "excessive subdirectory\n", dir, fn);
+ if (0 == use_all)
+ continue;
+ }
buf[0] = '\0';
strlcat(buf, dir, MAXPATHLEN);
@@ -1513,21 +1602,35 @@ ofile_dirbuild(const char *dir, const ch
sz = strlcat(buf, fn, MAXPATHLEN);
if (MAXPATHLEN <= sz) {
- fprintf(stderr, "%s: Path too long\n", dir);
- return(0);
+ if (verb) fprintf(stderr, "%s/%s: "
+ "path too long\n", dir, fn);
+ continue;
}
-
+
if (verb > 2)
- printf("%s: Scanning\n", buf);
+ printf("%s: scanning\n", buf);
ofile_dirbuild(buf, sec, arch, src_form, of);
+ continue;
}
- if (DT_REG != dp->d_type ||
- (NULL == psec && !use_all) ||
- ! strcmp(MANDOC_DB, fn) ||
- ! strcmp(MANDOC_IDX, fn))
+ if (DT_REG != dp->d_type) {
+ if (verb)
+ fprintf(stderr,
+ "%s/%s: not a regular file\n",
+ dir, fn);
+ continue;
+ }
+ if (!strcmp(MANDOC_DB, fn) || !strcmp(MANDOC_IDX, fn))
continue;
+ if ('\0' == *psec) {
+ if (verb)
+ fprintf(stderr,
+ "%s/%s: file outside section\n",
+ dir, fn);
+ if (0 == use_all)
+ continue;
+ }
/*
* By default, skip files where the file name suffix
@@ -1536,23 +1639,29 @@ ofile_dirbuild(const char *dir, const ch
*/
suffix = strrchr(fn, '.');
- if (0 == use_all) {
- if (NULL == suffix)
+ if (NULL == suffix) {
+ if (verb)
+ fprintf(stderr,
+ "%s/%s: no filename suffix\n",
+ dir, fn);
+ if (0 == use_all)
continue;
- if ((MANDOC_SRC & src_form &&
- strcmp(suffix + 1, psec)) ||
+ } else if ((MANDOC_SRC & src_form &&
+ strcmp(suffix + 1, psec)) ||
(MANDOC_FORM & src_form &&
- strcmp(suffix + 1, "0")))
- continue;
- }
- if (NULL != suffix) {
+ strcmp(suffix + 1, "0"))) {
+ if (verb)
+ fprintf(stderr,
+ "%s/%s: wrong filename suffix\n",
+ dir, fn);
+ if (0 == use_all)
+ continue;
if ('0' == suffix[1])
src_form |= MANDOC_FORM;
else if ('1' <= suffix[1] && '9' >= suffix[1])
src_form |= MANDOC_SRC;
}
-
/*
* Skip formatted manuals if a source version is
* available. Ignore the age: it is very unlikely
@@ -1561,11 +1670,11 @@ ofile_dirbuild(const char *dir, const ch
* and in ports, old manuals get removed on update.
*/
if (0 == use_all && MANDOC_FORM & src_form &&
- NULL != psec) {
+ '\0' != *psec) {
buf[0] = '\0';
strlcat(buf, dir, MAXPATHLEN);
p = strrchr(buf, '/');
- if (NULL != parch && NULL != p)
+ if ('\0' != *parch && NULL != p)
for (p--; p > buf; p--)
if ('/' == *p)
break;
@@ -1578,7 +1687,8 @@ ofile_dirbuild(const char *dir, const ch
strlcat(buf, "/", MAXPATHLEN);
sz = strlcat(buf, fn, MAXPATHLEN);
if (sz >= MAXPATHLEN) {
- fprintf(stderr, "%s: Path too long\n", buf);
+ if (verb) fprintf(stderr, "%s/%s: "
+ "path too long\n", dir, fn);
continue;
}
q = strrchr(buf, '.');
@@ -1586,8 +1696,8 @@ ofile_dirbuild(const char *dir, const ch
*q = '\0';
sz = strlcat(buf, psec, MAXPATHLEN);
if (sz >= MAXPATHLEN) {
- fprintf(stderr,
- "%s: Path too long\n", buf);
+ if (verb) fprintf(stderr, "%s/%s: "
+ "path too long\n", dir, fn);
continue;
}
if (0 == access(buf, R_OK))
@@ -1595,23 +1705,22 @@ ofile_dirbuild(const char *dir, const ch
}
}
- assert('.' == dir[0]);
- assert('/' == dir[1]);
buf[0] = '\0';
- strlcat(buf, dir + 2, MAXPATHLEN);
- strlcat(buf, "/", MAXPATHLEN);
+ assert('.' == dir[0]);
+ if ('/' == dir[1]) {
+ strlcat(buf, dir + 2, MAXPATHLEN);
+ strlcat(buf, "/", MAXPATHLEN);
+ }
sz = strlcat(buf, fn, MAXPATHLEN);
if (sz >= MAXPATHLEN) {
- fprintf(stderr, "%s: Path too long\n", dir);
+ fprintf(stderr, "%s/%s: path too long\n", dir, fn);
continue;
}
nof = mandoc_calloc(1, sizeof(struct of));
nof->fname = mandoc_strdup(buf);
- if (NULL != psec)
- nof->sec = mandoc_strdup(psec);
- if (NULL != parch)
- nof->arch = mandoc_strdup(parch);
+ nof->sec = mandoc_strdup(psec);
+ nof->arch = mandoc_strdup(parch);
nof->src_form = src_form;
/*
@@ -1628,7 +1737,7 @@ ofile_dirbuild(const char *dir, const ch
*/
if (verb > 2)
- printf("%s: Scheduling\n", buf);
+ printf("%s: scheduling\n", buf);
if (NULL == *of) {
*of = nof;
(*of)->first = nof;
@@ -1656,15 +1765,4 @@ ofile_free(struct of *of)
free(of);
of = nof;
}
-}
-
-static void
-usage(void)
-{
-
- fprintf(stderr, "usage: %s [-v] "
- "[-C file] |"
- " dir ... |"
- " -d dir [file ...] |"
- " -u dir [file ...]\n", progname);
}
--
To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: implement mandocdb -t
2011-12-24 16:46 implement mandocdb -t Ingo Schwarze
@ 2011-12-24 23:20 ` Kristaps Dzonsons
2011-12-25 0:38 ` Ingo Schwarze
0 siblings, 1 reply; 4+ messages in thread
From: Kristaps Dzonsons @ 2011-12-24 23:20 UTC (permalink / raw)
To: tech
Hi Ingo,
I have a few issues with this patch -- not the intent or code, but
rather with the consistency of the verbosity and warnings.
First, for consistency, all warnings/errors should follow "CAUSE:
REASON" -- including those in the getopt() switch. Thus, "conflicting
options" messages should be prefixed by the offending switch case and
"Too many arguments" should begin with "-C".
Second, verbosity. I think we should settle this now. BSD.lv only has
a single `-v' and I think this should stay as such ("%s: added to index"
or "%s: removed from index" (or whatever the message is)). This is
completely clear and is also valuable to the user. Noting keywords
added or removed is unnecessary when you consider that mandocdb(8) isn't
there to debug the keywords of individual manuals: it's there to index them.
Regarding errors, the existences of (and not reasons for) mandoc(3)
parse errors are printed to stderr ("parse failed"). For catpages, I
think it's unnecessary to print anything more than the same. After all,
it's not like the operator can do anything about the page, no? Just
give the same error regardless the reason. Maybe an extra kick like
"%s: parse failed (not a manpage?)".
Moving on to the warnings (wrong sections, architectures, etc.). This
shouldn't be a `-v', as this isn't a verbose message, it's a warning.
Maybe we can pull in `-W' from mandoc(1)? I don't think these should be
printed by default, as they don't affect parse status. An extra
"warning" flag is both consistent with other utilities and doesn't
confuse the utility of `-v'.
Another option is to print the warnings by default, but use `-q' to
silence them. But that's also confusing because one `-q' silences
warnings, but `-qq' silences parse errors also? Ugh...
If we follow this logic, then `-t' is much clearer, as it does nothing
but run in `add' mode without changing the database. (Isn't this
usually termed `-n', as with make(1)? Sigh.) However, the `-W' flag
(or whatever it becomes) could be turned on by default just like
`-Tlint' turns on `-Wall' in mandoc(1). `-v' would be unaffected and
meaningless as no files are actually added or removed.
Thoughts?
Kristaps
--
To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: implement mandocdb -t
2011-12-24 23:20 ` Kristaps Dzonsons
@ 2011-12-25 0:38 ` Ingo Schwarze
2011-12-25 0:56 ` Kristaps Dzonsons
0 siblings, 1 reply; 4+ messages in thread
From: Ingo Schwarze @ 2011-12-25 0:38 UTC (permalink / raw)
To: tech
Hi Kristaps,
thanks for the quick review!
Kristaps Dzonsons wrote on Sun, Dec 25, 2011 at 01:20:15AM +0200:
> I have a few issues with this patch -- not the intent or code, but
> rather with the consistency of the verbosity and warnings.
>
> First, for consistency, all warnings/errors should follow "CAUSE:
> REASON" -- including those in the getopt() switch. Thus,
> "conflicting options" messages should be prefixed by the offending
> switch case and "Too many arguments" should begin with "-C".
Makes sense to me; fixed.
> Second, verbosity. I think we should settle this now. BSD.lv only
> has a single `-v' and I think this should stay as such ("%s: added
> to index" or "%s: removed from index" (or whatever the message is)).
> This is completely clear and is also valuable to the user. Noting
> keywords added or removed is unnecessary when you consider that
> mandocdb(8) isn't there to debug the keywords of individual manuals:
> it's there to index them.
Indeed, i didn't put dumping of keywords back.
I'm ok with having only a single -v, both in the documentation and
in the long term. For now, i'd like to keep the "scanning" and
"scheduling" messages around, they help tremendously for
debugging, but can be removed when the dust has settled.
> Regarding errors, the existences of (and not reasons for) mandoc(3)
> parse errors are printed to stderr ("parse failed"). For catpages,
> I think it's unnecessary to print anything more than the same.
> After all, it's not like the operator can do anything about the
> page, no? Just give the same error regardless the reason. Maybe an
> extra kick like "%s: parse failed (not a manpage?)".
Well, mandocdb -t is not for the operator, but for the porter,
and for the porter, the message should be as specific as possible,
because a diligent porter may wish to fix the manuals and send
patches upstream.
> Moving on to the warnings (wrong sections, architectures, etc.).
> This shouldn't be a `-v', as this isn't a verbose message, it's a
> warning. Maybe we can pull in `-W' from mandoc(1)? I don't think
> these should be printed by default, as they don't affect parse
> status.
Definitely, because printing them by default would clobber
the output of pkg_add(1), and espie@ would not be amused.
> An extra "warning" flag is both consistent with other
> utilities and doesn't confuse the utility of `-v'.
Interesting idea.
Right now, makewhatis(8) doesn't have a -W flag.
The only use of -W i can find is in Linux man(1):
-W [Linux man] list the pathnames of matching formatted pages
which is similar to man -w, just for formatted manuals.
I don't think we have to respect that.
> Another option is to print the warnings by default, but use `-q' to
> silence them.
No way: pkg_add(1)!
> But that's also confusing because one `-q' silences
> warnings, but `-qq' silences parse errors also? Ugh...
Ugh indeed.
> If we follow this logic, then `-t' is much clearer, as it does
> nothing but run in `add' mode without changing the database. (Isn't
> this usually termed `-n', as with make(1)? Sigh.)
Well, both OpenBSD makewhatis(8) and Linux mandb(8) call it -t,
so i'd like to stick with that.
> However, the `-W' flag (or whatever it becomes) could be turned on
> by default just like `-Tlint' turns on `-Wall' in mandoc(1).
Indeed, -t without -W doesn't make much sense.
Besides, -t has to imply -W because otherwise,
we would break established behaviour of makewhatis -t.
> `-v' would be unaffected and meaningless as no files are actually
> added or removed.
>
> Thoughts?
Lots of useful input, see below for a patch to be applied
on top of the previous one, for easier review.
OK in that revised form?
Ingo
--- mandocdb.8.orig
+++ mandocdb.8
@@ -22,21 +22,20 @@
.Nd index UNIX manuals
.Sh SYNOPSIS
.Nm
-.Op Fl avvv
+.Op Fl avW
.Op Fl C Ar file
.Nm
-.Op Fl avvv
+.Op Fl avW
.Ar dir ...
.Nm
-.Op Fl vvv
+.Op Fl vW
.Fl d Ar dir
.Op Ar
.Nm
-.Op Fl vvv
+.Op Fl vW
.Fl u Ar dir
.Op Ar
.Nm
-.Op Fl vv
.Fl t Ar
.Sh DESCRIPTION
The
@@ -98,8 +97,8 @@ for potential problems.
No databases are modified.
Implies
.Fl a
-and one
-.Fl v .
+and
+.Fl W .
All diagnostic messages are printed to the standard output;
the standard error output is not used.
.It Fl u Ar dir
@@ -109,19 +108,10 @@ from the database in
.Ar dir
without truncating it.
.It Fl v
-Verbose operation; can be specified multiple times.
-By default,
-.Nm
-operates silently.
-One
-.Fl v
-warns about potential problems.
-A second
-.Fl v
-displays all files added to or removed from the index.
-A third
-.Fl v
-displays additional debugging information.
+Display all files added or removed to the index.
+.It Fl W
+Print warnings about potential problems with manual pages
+to the standard error output.
.El
.Pp
If fatal parse errors are encountered while parsing, the offending file
--- mandocdb.c.orig
+++ mandocdb.c
@@ -267,6 +267,7 @@ static const struct mdoc_handler mdocs[MDOC_MAX] = {
static const char *progname;
static int use_all; /* Use all directories and files. */
static int verb; /* Output verbosity level. */
+static int warnings; /* Potential problems in manuals. */
int
mandocdb(int argc, char *argv[])
@@ -299,22 +300,24 @@ mandocdb(int argc, char *argv[])
memset(&mdb, 0, sizeof(struct mdb));
memset(&recs, 0, sizeof(struct recs));
- verb = 0;
use_all = 0;
+ verb = 0;
+ warnings = 0;
of = NULL;
mp = NULL;
hash = NULL;
op = OP_DEFAULT;
dir = NULL;
- while (-1 != (ch = getopt(argc, argv, "aC:d:tu:v")))
+ while (-1 != (ch = getopt(argc, argv, "aC:d:tu:vW")))
switch (ch) {
case ('a'):
use_all = 1;
break;
case ('C'):
if (op) {
- fprintf(stderr, "conflicting options\n");
+ fprintf(stderr,
+ "-C: conflicting options\n");
goto usage;
}
dir = optarg;
@@ -322,7 +325,8 @@ mandocdb(int argc, char *argv[])
break;
case ('d'):
if (op) {
- fprintf(stderr, "conflicting options\n");
+ fprintf(stderr,
+ "-d: conflicting options\n");
goto usage;
}
dir = optarg;
@@ -331,16 +335,18 @@ mandocdb(int argc, char *argv[])
case ('t'):
dup2(STDOUT_FILENO, STDERR_FILENO);
if (op) {
- fprintf(stderr, "conflicting options\n");
+ fprintf(stderr,
+ "-t: conflicting options\n");
goto usage;
}
op = OP_TEST;
use_all = 1;
- verb++;
+ warnings = 1;
break;
case ('u'):
if (op) {
- fprintf(stderr, "conflicting options\n");
+ fprintf(stderr,
+ "-u: conflicting options\n");
goto usage;
}
dir = optarg;
@@ -349,6 +355,9 @@ mandocdb(int argc, char *argv[])
case ('v'):
verb++;
break;
+ case ('W'):
+ warnings = 1;
+ break;
default:
goto usage;
}
@@ -357,7 +366,7 @@ mandocdb(int argc, char *argv[])
argv += optind;
if (OP_CONFFILE == op && argc > 0) {
- fprintf(stderr, "too many arguments for -C\n");
+ fprintf(stderr, "-C: too many arguments\n");
goto usage;
}
@@ -489,11 +498,6 @@ mandocdb(int argc, char *argv[])
exit((int)MANDOCLEVEL_SYSERR);
}
- if (verb > 2) {
- printf("%s: truncated\n", mdb.dbn);
- printf("%s: truncated\n", mdb.idxn);
- }
-
ofile_free(of);
of = NULL;
@@ -610,7 +614,7 @@ index_merge(const struct of *of, struct mparse *mp,
assert(of->sec);
assert(msec);
if (strcasecmp(msec, of->sec)) {
- if (verb)
+ if (warnings)
fprintf(stderr, "%s: "
"section \"%s\" manual "
"in \"%s\" directory\n",
@@ -621,7 +625,7 @@ index_merge(const struct of *of, struct mparse *mp,
assert(of->arch);
assert(march);
if (strcasecmp(march, of->arch)) {
- if (verb)
+ if (warnings)
fprintf(stderr, "%s: "
"architecture \"%s\" manual "
"in \"%s\" directory\n",
@@ -639,7 +643,7 @@ index_merge(const struct of *of, struct mparse *mp,
assert(of->title);
assert(mtitle);
if (strcasecmp(mtitle, of->title)) {
- if (verb)
+ if (warnings)
fprintf(stderr, "%s: "
"title \"%s\" in file "
"but \"%s\" in filename\n",
@@ -737,7 +741,7 @@ index_merge(const struct of *of, struct mparse *mp,
val.data = dbuf->cp;
val.size = dbuf->len;
- if (verb > 1)
+ if (verb)
printf("%s: adding to index\n", fn);
dbt_put(mdb->idx, mdb->idxn, &key, &val);
@@ -825,7 +829,7 @@ index_prune(const struct of *ofile, struct mdb *mdb, struct recs *recs)
exit((int)MANDOCLEVEL_SYSERR);
}
- if (verb > 1)
+ if (verb)
printf("%s: deleting from index\n", fn);
val.size = 0;
@@ -1344,7 +1348,7 @@ pformatted(DB *hash, struct buf *buf, struct buf *dbuf,
size_t len, plen;
if (NULL == (stream = fopen(of->fname, "r"))) {
- if (verb)
+ if (warnings)
perror(of->fname);
return;
}
@@ -1384,7 +1388,7 @@ pformatted(DB *hash, struct buf *buf, struct buf *dbuf,
line = fgetln(stream, &len);
if (NULL == line || ' ' != *line || '\n' != line[(int)len - 1]) {
- if (verb)
+ if (warnings)
fprintf(stderr, "%s: cannot find NAME section\n",
of->fname);
buf_appendb(dbuf, buf->cp, buf->size);
@@ -1405,7 +1409,7 @@ pformatted(DB *hash, struct buf *buf, struct buf *dbuf,
for (p += 2; ' ' == *p || '\b' == *p; p++)
/* Skip to next word. */ ;
} else {
- if (verb)
+ if (warnings)
fprintf(stderr, "%s: no dash in title line\n",
of->fname);
p = line;
@@ -1485,7 +1489,7 @@ ofile_argbuild(int argc, char *argv[], struct of **of)
break;
}
if ('\0' == *title) {
- if (verb)
+ if (warnings)
fprintf(stderr,
"%s: cannot deduce title "
"from filename\n",
@@ -1508,7 +1512,7 @@ ofile_argbuild(int argc, char *argv[], struct of **of)
* Add the structure to the list.
*/
- if (verb > 2)
+ if (verb > 1)
printf("%s: scheduling\n", argv[i]);
if (NULL == *of) {
*of = nof;
@@ -1543,7 +1547,7 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
int src_form;
if (NULL == (d = opendir(dir))) {
- if (verb)
+ if (warnings)
perror(dir);
return;
}
@@ -1574,8 +1578,9 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
src_form |= MANDOC_FORM;
sec = fn + 3;
} else {
- if (verb) fprintf(stderr, "%s/%s: "
- "bad section\n", dir, fn);
+ if (warnings) fprintf(stderr,
+ "%s/%s: bad section\n",
+ dir, fn);
if (use_all)
sec = fn;
else
@@ -1583,14 +1588,15 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
}
} else if ('\0' == *arch) {
if (NULL != strchr(fn, '.')) {
- if (verb) fprintf(stderr, "%s/%s: "
- "bad architecture\n", dir, fn);
+ if (warnings) fprintf(stderr,
+ "%s/%s: bad architecture\n",
+ dir, fn);
if (0 == use_all)
continue;
}
arch = fn;
} else {
- if (verb) fprintf(stderr, "%s/%s: "
+ if (warnings) fprintf(stderr, "%s/%s: "
"excessive subdirectory\n", dir, fn);
if (0 == use_all)
continue;
@@ -1602,12 +1608,12 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
sz = strlcat(buf, fn, MAXPATHLEN);
if (MAXPATHLEN <= sz) {
- if (verb) fprintf(stderr, "%s/%s: "
+ if (warnings) fprintf(stderr, "%s/%s: "
"path too long\n", dir, fn);
continue;
}
- if (verb > 2)
+ if (verb > 1)
printf("%s: scanning\n", buf);
ofile_dirbuild(buf, sec, arch, src_form, of);
@@ -1615,7 +1621,7 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
}
if (DT_REG != dp->d_type) {
- if (verb)
+ if (warnings)
fprintf(stderr,
"%s/%s: not a regular file\n",
dir, fn);
@@ -1624,7 +1630,7 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
if (!strcmp(MANDOC_DB, fn) || !strcmp(MANDOC_IDX, fn))
continue;
if ('\0' == *psec) {
- if (verb)
+ if (warnings)
fprintf(stderr,
"%s/%s: file outside section\n",
dir, fn);
@@ -1640,7 +1646,7 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
suffix = strrchr(fn, '.');
if (NULL == suffix) {
- if (verb)
+ if (warnings)
fprintf(stderr,
"%s/%s: no filename suffix\n",
dir, fn);
@@ -1650,7 +1656,7 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
strcmp(suffix + 1, psec)) ||
(MANDOC_FORM & src_form &&
strcmp(suffix + 1, "0"))) {
- if (verb)
+ if (warnings)
fprintf(stderr,
"%s/%s: wrong filename suffix\n",
dir, fn);
@@ -1687,8 +1693,9 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
strlcat(buf, "/", MAXPATHLEN);
sz = strlcat(buf, fn, MAXPATHLEN);
if (sz >= MAXPATHLEN) {
- if (verb) fprintf(stderr, "%s/%s: "
- "path too long\n", dir, fn);
+ if (warnings) fprintf(stderr,
+ "%s/%s: path too long\n",
+ dir, fn);
continue;
}
q = strrchr(buf, '.');
@@ -1696,8 +1703,9 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
*q = '\0';
sz = strlcat(buf, psec, MAXPATHLEN);
if (sz >= MAXPATHLEN) {
- if (verb) fprintf(stderr, "%s/%s: "
- "path too long\n", dir, fn);
+ if (warnings) fprintf(stderr,
+ "%s/%s: path too long\n",
+ dir, fn);
continue;
}
if (0 == access(buf, R_OK))
@@ -1713,7 +1721,8 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
}
sz = strlcat(buf, fn, MAXPATHLEN);
if (sz >= MAXPATHLEN) {
- fprintf(stderr, "%s/%s: path too long\n", dir, fn);
+ if (warnings) fprintf(stderr,
+ "%s/%s: path too long\n", dir, fn);
continue;
}
@@ -1736,7 +1745,7 @@ ofile_dirbuild(const char *dir, const char* psec, const char *parch,
* Add the structure to the list.
*/
- if (verb > 2)
+ if (verb > 1)
printf("%s: scheduling\n", buf);
if (NULL == *of) {
*of = nof;
--
To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: implement mandocdb -t
2011-12-25 0:38 ` Ingo Schwarze
@ 2011-12-25 0:56 ` Kristaps Dzonsons
0 siblings, 0 replies; 4+ messages in thread
From: Kristaps Dzonsons @ 2011-12-25 0:56 UTC (permalink / raw)
To: tech
On 25/12/2011 02:38, Ingo Schwarze wrote:
> Hi Kristaps,
>
> thanks for the quick review!
>
> Kristaps Dzonsons wrote on Sun, Dec 25, 2011 at 01:20:15AM +0200:
>
>> I have a few issues with this patch -- not the intent or code, but
>> rather with the consistency of the verbosity and warnings.
>>
>> First, for consistency, all warnings/errors should follow "CAUSE:
>> REASON" -- including those in the getopt() switch. Thus,
>> "conflicting options" messages should be prefixed by the offending
>> switch case and "Too many arguments" should begin with "-C".
>
> Makes sense to me; fixed.
>
>> Second, verbosity. I think we should settle this now. BSD.lv only
>> has a single `-v' and I think this should stay as such ("%s: added
>> to index" or "%s: removed from index" (or whatever the message is)).
>> This is completely clear and is also valuable to the user. Noting
>> keywords added or removed is unnecessary when you consider that
>> mandocdb(8) isn't there to debug the keywords of individual manuals:
>> it's there to index them.
>
> Indeed, i didn't put dumping of keywords back.
>
> I'm ok with having only a single -v, both in the documentation and
> in the long term. For now, i'd like to keep the "scanning" and
> "scheduling" messages around, they help tremendously for
> debugging, but can be removed when the dust has settled.
>
>> Regarding errors, the existences of (and not reasons for) mandoc(3)
>> parse errors are printed to stderr ("parse failed"). For catpages,
>> I think it's unnecessary to print anything more than the same.
>> After all, it's not like the operator can do anything about the
>> page, no? Just give the same error regardless the reason. Maybe an
>> extra kick like "%s: parse failed (not a manpage?)".
>
>
>> Moving on to the warnings (wrong sections, architectures, etc.).
>> This shouldn't be a `-v', as this isn't a verbose message, it's a
>> warning. Maybe we can pull in `-W' from mandoc(1)? I don't think
>> these should be printed by default, as they don't affect parse
>> status.
>
> Definitely, because printing them by default would clobber
> the output of pkg_add(1), and espie@ would not be amused.
>
>> An extra "warning" flag is both consistent with other
>> utilities and doesn't confuse the utility of `-v'.
>
> Interesting idea.
> Right now, makewhatis(8) doesn't have a -W flag.
>
> The only use of -W i can find is in Linux man(1):
> -W [Linux man] list the pathnames of matching formatted pages
> which is similar to man -w, just for formatted manuals.
> I don't think we have to respect that.
>
>> Another option is to print the warnings by default, but use `-q' to
>> silence them.
>
> No way: pkg_add(1)!
>
>> But that's also confusing because one `-q' silences
>> warnings, but `-qq' silences parse errors also? Ugh...
>
> Ugh indeed.
>
>> If we follow this logic, then `-t' is much clearer, as it does
>> nothing but run in `add' mode without changing the database. (Isn't
>> this usually termed `-n', as with make(1)? Sigh.)
>
> Well, both OpenBSD makewhatis(8) and Linux mandb(8) call it -t,
> so i'd like to stick with that.
>
>> However, the `-W' flag (or whatever it becomes) could be turned on
>> by default just like `-Tlint' turns on `-Wall' in mandoc(1).
>
> Indeed, -t without -W doesn't make much sense.
> Besides, -t has to imply -W because otherwise,
> we would break established behaviour of makewhatis -t.
>
>> `-v' would be unaffected and meaningless as no files are actually
>> added or removed.
>>
>> Thoughts?
>
> Lots of useful input, see below for a patch to be applied
> on top of the previous one, for easier review.
>
> OK in that revised form?
Ingo,
Thanks for making these changes so quickly! I like what I see, so
please commit. You don't, by the way, need to initialise the static
variables to zero.
> Well, mandocdb -t is not for the operator, but for the porter,
> and for the porter, the message should be as specific as possible,
> because a diligent porter may wish to fix the manuals and send
> patches upstream.
I agree with this logic.
When you've checked everything in, by the way, I've a local patch that
groks everything between NAME and the next section instead of just the
first line. This catches full descriptions and all names. I'll convert
that to a function reading into a string buffer, then fit it into your
checks.
Thanks again,
Kristaps
--
To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-25 0:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-24 16:46 implement mandocdb -t Ingo Schwarze
2011-12-24 23:20 ` Kristaps Dzonsons
2011-12-25 0:38 ` Ingo Schwarze
2011-12-25 0:56 ` Kristaps Dzonsons
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).