A few things: 1. create a savefd function, so that if you have a file open, you can call this function instead of closing and reopening it 2. argv0 incorrectly used, change it to name instead 3. don't say "we don't accept executable attachments" when it's really an internal error 4. The big one: don't free buf, since you use it for strdup below. mktemp does not do a strdup; smprint does. So the odds of trashing name, once you free buf, is pretty good. Notes: Tue Jun 12 14:59:56 EDT 2007 geoff I think that the current vf (newer than the one used as the base of this diff) is fine. In particular, the use of argv0 is correct. The file name in question should appear in errstr. Reference: /n/sources/patch/sorry/vf-errors-try2 Date: Tue Jun 12 02:46:09 CES 2007 Signed-off-by: rminnich@gmail.com Reviewed-by: geoff --- /sys/src/cmd/upas/vf/vf.c Tue Jun 12 02:45:56 2007 +++ /sys/src/cmd/upas/vf/vf.c Tue Jun 12 02:45:53 2007 @@ -159,7 +159,7 @@ refuse(char *reason) { static char msg[] = - "mail refused: we don't accept executable attachments"; + "mail refused:"; postnote(PNGROUP, getpid(), smprint("%s: %s", msg, reason)); exits(msg); @@ -224,7 +224,7 @@ p->filename?s_to_c(p->filename):"?"); fprint(2, "The mail contained an executable attachment.\n"); fprint(2, "We refuse all mail containing such.\n"); - refuse(nil); + refuse("Executable attachment"); } np = problemchild(p); if(np != p) @@ -365,19 +365,14 @@ * save the message somewhere */ static vlong bodyoff; /* clumsy hack */ -static int -save(Part *p, char *file) +static int +savefd(Part *p, int fd) { - int fd; char *cp; + seek(fd, 0, 2); Bterm(&out); memset(&out, 0, sizeof(out)); - - fd = open(file, OWRITE); - if(fd < 0) - return -1; - seek(fd, 0, 2); Binit(&out, fd, OWRITE); cp = ctime(time(0)); cp[28] = 0; @@ -394,37 +389,53 @@ return 0; } +static int +save(Part *p, char *file) +{ + int fd; + + fd = open(file, OWRITE); + if(fd < 0) + return -1; + + return savefd(p, fd); + +} + /* * write to a file but save the fd for passbody. */ static char* savetmp(Part *p) { - char *name; + char *buf, *name, *retname; int fd; - name = mktemp(smprint("%s/vf.XXXXXXXXXXX", UPASTMP)); + buf = smprint("%s/vf.XXXXXXXXXXX", UPASTMP); + name = mktemp(buf); if((fd = create(name, OWRITE|OEXCL, 0666)) < 0){ - fprint(2, "%s: error creating temporary file: %r\n", argv0); + fprint(2, "%s(buf=%s): error creating temporary file: %r\n", name, buf); refuse("can't create temporary file"); } - close(fd); - if(save(p, name) < 0){ - fprint(2, "%s: error saving temporary file: %r\n", argv0); + if(savefd(p, fd) < 0){ + fprint(2, "%s: error saving temporary file: %r\n", name); refuse("can't write temporary file"); } + close(fd); if(p->tmpbuf){ fprint(2, "%s: error in savetmp: already have tmp file!\n", - argv0); + name); refuse("already have temporary file"); } p->tmpbuf = Bopen(name, OREAD|ORCLOSE); if(p->tmpbuf == nil){ - fprint(2, "%s: error reading temporary file: %r\n", argv0); + fprint(2, "%s: error reading temporary file: %r\n", name); refuse("error reading temporary file"); } Bseek(p->tmpbuf, bodyoff, 0); - return name; + retname = strdup(name); + free(buf); + return retname; } /*