Hi George,
George Brown wrote on Thu, Aug 24, 2017 at 02:01:05PM +0100:
> In mandocdb.c it appears cmp(1) and rm(1) are executed in a child
> process.
Indeed, i never really liked that, yet i didn't spend much time
thinking about it either.
> It seems that if the logic from these programs were duplicated
> the pledge in mandocdb.c could be further restricted and even not
> bother with forking.
You have a point that the need for the "proc exec" pledges is ugly
in particular, and it would be nice to get rid of those pledges.
> Would such a change be pointless churn however?
At least the removal of the forking for "rm -rf" was not
pointless churn. Simplification is never pointless.
> Both cmp(1) and rm(1)
> are simple programs and are pledge'd themselves. Not to mention the
> creation of the mandoc database is in itself a short lived process.
>
> To be clear I'm not proposing a change (indeed I have no diff) but
> rather I am simply curious to the opinion of others in the OpenBSD
> community.
See below for what i committed so far. I shall look into removing
the fork and exec for cmp(1) as well. I'm not completely sure yet
that it will work out, but i suspect it won't make the code
substantially longer and only duplicate such a minimal and simple
part of cmp(1) that the duplication won't matter, and be outweighed
by the stricter pledge.
Thanks for reading the code!
Ingo
Log Message:
-----------
No need to fork and exec rm(1) -rf, we know that we have exactly
one file and exactly one directory to remove. While here, increase
the size of the buffer such that the file name actually fits.
Minus 17 lines of code, no functional change.
Opportunity for simplification reported by George Brown <321 dot
george at gmail dot com> on misc@.
Modified Files:
--------------
mandoc:
mandocdb.c
Revision Data
-------------
Index: mandocdb.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/mandocdb.c,v
retrieving revision 1.253
retrieving revision 1.254
diff -Lmandocdb.c -Lmandocdb.c -u -p -r1.253 -r1.254
--- mandocdb.c
+++ mandocdb.c
@@ -2119,7 +2119,7 @@ dbprune(struct dba *dba)
static void
dbwrite(struct dba *dba)
{
- char tfn[32];
+ char tfn[33];
int status;
pid_t child;
@@ -2193,26 +2193,9 @@ dbwrite(struct dba *dba)
}
out:
+ unlink(tfn);
*strrchr(tfn, '/') = '\0';
- switch (child = fork()) {
- case -1:
- exitcode = (int)MANDOCLEVEL_SYSERR;
- say("", "&fork rm");
- return;
- case 0:
- execlp("rm", "rm", "-rf", tfn, (char *)NULL);
- say("", "&exec rm");
- exit((int)MANDOCLEVEL_SYSERR);
- default:
- break;
- }
- if (waitpid(child, &status, 0) == -1) {
- exitcode = (int)MANDOCLEVEL_SYSERR;
- say("", "&wait rm");
- } else if (WIFSIGNALED(status) || WEXITSTATUS(status)) {
- exitcode = (int)MANDOCLEVEL_SYSERR;
- say("", "%s: Cannot remove temporary directory", tfn);
- }
+ rmdir(tfn);
}
static int
