PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan 

Index: bin/csh/dol.c
===
RCS file: /cvs/src/bin/csh/dol.c,v
retrieving revision 1.17
diff -u -p -d -r1.17 dol.c
--- bin/csh/dol.c   12 Aug 2010 02:00:27 -  1.17
+++ bin/csh/dol.c   11 Jul 2014 09:12:11 -
@@ -829,7 +829,8 @@ heredoc(Char *term)
 
 if (mkstemp(tmp)  0)
stderror(ERR_SYSTEM, tmp, strerror(errno));
-(void) unlink(tmp);/* 0 0 inode! */
+else
+   (void) unlink(tmp); /* 0 0 inode! */
 Dv[0] = term;
 Dv[1] = NULL;
 gflag = 0;
Index: sbin/disklabel/disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.195
diff -u -p -d -r1.195 disklabel.c
--- sbin/disklabel/disklabel.c  5 May 2014 16:33:34 -   1.195
+++ sbin/disklabel/disklabel.c  11 Jul 2014 09:13:43 -
@@ -815,9 +815,12 @@ edit(struct disklabel *lp, int f)
FILE *fp;
u_int64_t total_sectors, starting_sector, ending_sector;
 
-   if ((fd = mkstemp(tmpfil)) == -1 || (fp = fdopen(fd, w)) == NULL) {
-   if (fd != -1)
+   if ((fd = mkstemp(tmpfil)) == -1 ||
+   (fp = fdopen(fd, w)) == NULL) {
+   if (fd != -1) {
+   unlink(tmpfil);
close(fd);
+   }
warn(%s, tmpfil);
return (1);
}
Index: sbin/scsi/scsi.c
===
RCS file: /cvs/src/sbin/scsi/scsi.c,v
retrieving revision 1.28
diff -u -p -d -r1.28 scsi.c
--- sbin/scsi/scsi.c12 Nov 2013 04:59:02 -  1.28
+++ sbin/scsi/scsi.c11 Jul 2014 09:13:44 -
@@ -571,8 +571,11 @@ edit_init(void)
strlcpy(edit_name, /var/tmp/sc, sizeof edit_name);
if ((fd = mkstemp(edit_name)) == -1)
err(1, mkstemp);
-   if ( (edit_file = fdopen(fd, w+)) == 0)
+   if ( (edit_file = fdopen(fd, w+)) == 0) {
+   unlink(edit_name);
+   close(fd);
err(1, fdopen);
+   }
edit_opened = 1;
 
atexit(edit_done);
Index: usr.bin/gzsig/sign.c
===
RCS file: /cvs/src/usr.bin/gzsig/sign.c,v
retrieving revision 1.13
diff -u -p -d -r1.13 sign.c
--- usr.bin/gzsig/sign.c10 Mar 2013 10:36:57 -  1.13
+++ usr.bin/gzsig/sign.c11 Jul 2014 09:14:10 -
@@ -281,6 +281,7 @@ sign(int argc, char *argv[])
if ((fout = fdopen(fd, w)) == NULL) {
fprintf(stderr, Error opening %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
close(fd);
continue;
@@ -288,6 +289,7 @@ sign(int argc, char *argv[])
if (copy_permissions(fileno(fin), fd)  0) {
fprintf(stderr, Error initializing %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
fclose(fout);
continue;
Index: usr.bin/htpasswd/htpasswd.c
===
RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
retrieving revision 1.10
diff -u -p -d -r1.10 htpasswd.c
--- usr.bin/htpasswd/htpasswd.c 24 Mar 2014 20:33:01 -  1.10
+++ usr.bin/htpasswd/htpasswd.c 11 Jul 2014 09:14:10 -
@@ -164,8 +164,10 @@ main(int argc, char** argv)
if ((fd = mkstemp(tmpl)) == -1)
err(1, mkstemp);
 
-   if ((out = fdopen(fd, w+)) == NULL)
+   if ((out = fdopen(fd, w+)) == NULL) {
+   unlink(tmpl);
err(1, cannot open tempfile);
+   }
 
while ((linelen = getline(line, linesize, in))
!= -1) {
Index: usr.bin/m4/eval.c
===
RCS file: /cvs/src/usr.bin/m4/eval.c,v
retrieving revision 1.72
diff -u -p -d -r1.72 eval.c
--- usr.bin/m4/eval.c   28 Apr 2014 12:34:11 -  1.72
+++ usr.bin/m4/eval.c   11 Jul 2014 09:14:11 -
@@ -818,8 +818,11 @@ dodiv(int n)
char fname[] = _PATH_DIVNAME;
 
if ((fd = mkstemp(fname))  0 || 
-   (outfile[n] = fdopen(fd, w+)) == NULL)
+   (outfile[n] = fdopen(fd, w+)) == NULL) {
+   if (fd != -1)
+   unlink(fname);
err(1, %s: cannot divert, fname);
+   }
if (unlink(fname) == -1)
err(1, %s: cannot unlink, fname);
}
Index: usr.bin/rwall/rwall.c

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Ville Valkonen 
On 11 July 2014 12:41, Doug Hogan d...@acyclic.org wrote:

 Index: bin/csh/dol.c
 ===
 RCS file: /cvs/src/bin/csh/dol.c,v
 retrieving revision 1.17
 diff -u -p -d -r1.17 dol.c
 --- bin/csh/dol.c   12 Aug 2010 02:00:27 -  1.17
 +++ bin/csh/dol.c   11 Jul 2014 09:12:11 -
 @@ -829,7 +829,8 @@ heredoc(Char *term)

  if (mkstemp(tmp)  0)
 stderror(ERR_SYSTEM, tmp, strerror(errno));
 -(void) unlink(tmp);/* 0 0 inode! */
 +else
 +   (void) unlink(tmp); /* 0 0 inode! */
  Dv[0] = term;
  Dv[1] = NULL;
  gflag = 0;
 Index: sbin/disklabel/disklabel.c
 ===
 RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
 retrieving revision 1.195
 diff -u -p -d -r1.195 disklabel.c
 --- sbin/disklabel/disklabel.c  5 May 2014 16:33:34 -   1.195
 +++ sbin/disklabel/disklabel.c  11 Jul 2014 09:13:43 -
 @@ -815,9 +815,12 @@ edit(struct disklabel *lp, int f)
 FILE *fp;
 u_int64_t total_sectors, starting_sector, ending_sector;

 -   if ((fd = mkstemp(tmpfil)) == -1 || (fp = fdopen(fd, w)) == NULL) {
 -   if (fd != -1)
 +   if ((fd = mkstemp(tmpfil)) == -1 ||
 +   (fp = fdopen(fd, w)) == NULL) {
 +   if (fd != -1) {
 +   unlink(tmpfil);
 close(fd);
 +   }
 warn(%s, tmpfil);
 return (1);
 }
 Index: sbin/scsi/scsi.c
 ===
 RCS file: /cvs/src/sbin/scsi/scsi.c,v
 retrieving revision 1.28
 diff -u -p -d -r1.28 scsi.c
 --- sbin/scsi/scsi.c12 Nov 2013 04:59:02 -  1.28
 +++ sbin/scsi/scsi.c11 Jul 2014 09:13:44 -
 @@ -571,8 +571,11 @@ edit_init(void)
 strlcpy(edit_name, /var/tmp/sc, sizeof edit_name);
 if ((fd = mkstemp(edit_name)) == -1)
 err(1, mkstemp);
 -   if ( (edit_file = fdopen(fd, w+)) == 0)
 +   if ( (edit_file = fdopen(fd, w+)) == 0) {
 +   unlink(edit_name);
 +   close(fd);
 err(1, fdopen);
 +   }
 edit_opened = 1;

 atexit(edit_done);
 Index: usr.bin/gzsig/sign.c
 ===
 RCS file: /cvs/src/usr.bin/gzsig/sign.c,v
 retrieving revision 1.13
 diff -u -p -d -r1.13 sign.c
 --- usr.bin/gzsig/sign.c10 Mar 2013 10:36:57 -  1.13
 +++ usr.bin/gzsig/sign.c11 Jul 2014 09:14:10 -
 @@ -281,6 +281,7 @@ sign(int argc, char *argv[])
 if ((fout = fdopen(fd, w)) == NULL) {
 fprintf(stderr, Error opening %s: %s\n,
 tmppath, strerror(errno));
 +   unlink(tmppath);
 fclose(fin);
 close(fd);
 continue;
 @@ -288,6 +289,7 @@ sign(int argc, char *argv[])
 if (copy_permissions(fileno(fin), fd)  0) {
 fprintf(stderr, Error initializing %s: %s\n,
 tmppath, strerror(errno));
 +   unlink(tmppath);
 fclose(fin);
 fclose(fout);
 continue;
 Index: usr.bin/htpasswd/htpasswd.c
 ===
 RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
 retrieving revision 1.10
 diff -u -p -d -r1.10 htpasswd.c
 --- usr.bin/htpasswd/htpasswd.c 24 Mar 2014 20:33:01 -  1.10
 +++ usr.bin/htpasswd/htpasswd.c 11 Jul 2014 09:14:10 -
 @@ -164,8 +164,10 @@ main(int argc, char** argv)
 if ((fd = mkstemp(tmpl)) == -1)
 err(1, mkstemp);

 -   if ((out = fdopen(fd, w+)) == NULL)
 +   if ((out = fdopen(fd, w+)) == NULL) {
 +   unlink(tmpl);
 err(1, cannot open tempfile);
 +   }

 while ((linelen = getline(line, linesize, in))
 != -1) {
 Index: usr.bin/m4/eval.c
 ===
 RCS file: /cvs/src/usr.bin/m4/eval.c,v
 retrieving revision 1.72
 diff -u -p -d -r1.72 eval.c
 --- usr.bin/m4/eval.c   28 Apr 2014 12:34:11 -  1.72
 +++ usr.bin/m4/eval.c   11 Jul 2014 09:14:11 -
 @@ -818,8 +818,11 @@ dodiv(int n)
 char fname[] = _PATH_DIVNAME;

 if ((fd = mkstemp(fname))  0 ||
 -   (outfile[n] = fdopen(fd, w+)) == NULL)
 +   (outfile[n] = fdopen(fd, w+)) == NULL) {
 +   if (fd != -1)
 +   unlink(fname);
 err(1, %s: cannot divert,

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Philip Guenther 
On Fri, Jul 11, 2014 at 11:41 AM, Doug Hogan d...@acyclic.org wrote:

 Index: sbin/disklabel/disklabel.c
 ===
 RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
 retrieving revision 1.195
 diff -u -p -d -r1.195 disklabel.c
 --- sbin/disklabel/disklabel.c  5 May 2014 16:33:34 -   1.195
 +++ sbin/disklabel/disklabel.c  11 Jul 2014 09:13:43 -
 @@ -815,9 +815,12 @@ edit(struct disklabel *lp, int f)
 FILE *fp;
 u_int64_t total_sectors, starting_sector, ending_sector;

 -   if ((fd = mkstemp(tmpfil)) == -1 || (fp = fdopen(fd, w)) ==
 NULL) {
 -   if (fd != -1)
 +   if ((fd = mkstemp(tmpfil)) == -1 ||
 +   (fp = fdopen(fd, w)) == NULL) {
 +   if (fd != -1) {
 +   unlink(tmpfil);
 close(fd);
 +   }
 warn(%s, tmpfil);


This should call warn() before unlink() or close() to guarantee that the
correct errno value is reported.




 -   if ( (edit_file = fdopen(fd, w+)) == 0)
 +   if ( (edit_file = fdopen(fd, w+)) == 0) {
 +   unlink(edit_name);
 +   close(fd);
 err(1, fdopen);
 +   }


This and several other need to save errno and use errc(), ala:
if ( (edit_file = fdopen(fd, w+)) == 0) {
int saved_errno  = errno;
unlink(edit_name);
close(fd);
errc(1, saved_errno, fdopen);
}


Philip Guenther

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan 
On Fri, Jul 11, 2014 at 12:19:22PM +0200, Philip Guenther wrote:
 This should call warn() before unlink() or close() to guarantee that the
 correct errno value is reported.

Philip,

I see what you are saying.  I was following the man page example in
mkstemp(3) which calls warn() after unlink/close.  I'll update the
patch.

I see a number of places in the tree which call warn like the man page
example.  I'll submit a patch to fix those too.

Thanks!

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan 
On Fri, Jul 11, 2014 at 12:19:22PM +0200, Philip Guenther wrote:
 This should call warn() before unlink() or close() to guarantee that the
 correct errno value is reported.
...
 This and several other need to save errno and use errc(), ala:

Updated patch.  Updated mktemp.3 this time.


Index: bin/csh/dol.c
===
RCS file: /cvs/src/bin/csh/dol.c,v
retrieving revision 1.17
diff -u -p -d -r1.17 dol.c
--- bin/csh/dol.c   12 Aug 2010 02:00:27 -  1.17
+++ bin/csh/dol.c   11 Jul 2014 16:20:04 -
@@ -829,7 +829,8 @@ heredoc(Char *term)
 
 if (mkstemp(tmp)  0)
stderror(ERR_SYSTEM, tmp, strerror(errno));
-(void) unlink(tmp);/* 0 0 inode! */
+else
+   (void) unlink(tmp); /* 0 0 inode! */
 Dv[0] = term;
 Dv[1] = NULL;
 gflag = 0;
Index: lib/libc/stdio/mktemp.3
===
RCS file: /cvs/src/lib/libc/stdio/mktemp.3,v
retrieving revision 1.51
diff -u -p -d -r1.51 mktemp.3
--- lib/libc/stdio/mktemp.3 5 Jun 2013 03:39:23 -   1.51
+++ lib/libc/stdio/mktemp.3 11 Jul 2014 16:20:18 -
@@ -147,11 +147,11 @@ int fd;
 strlcpy(sfn, /tmp/ed.XX, sizeof(sfn));
 if ((fd = mkstemp(sfn)) == -1 ||
 (sfp = fdopen(fd, w+)) == NULL) {
+   warn(%s, sfn);
if (fd != -1) {
unlink(sfn);
close(fd);
}
-   warn(%s, sfn);
return (NULL);
 }
 return (sfp);
Index: sbin/disklabel/disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.195
diff -u -p -d -r1.195 disklabel.c
--- sbin/disklabel/disklabel.c  5 May 2014 16:33:34 -   1.195
+++ sbin/disklabel/disklabel.c  11 Jul 2014 16:20:22 -
@@ -815,10 +815,13 @@ edit(struct disklabel *lp, int f)
FILE *fp;
u_int64_t total_sectors, starting_sector, ending_sector;
 
-   if ((fd = mkstemp(tmpfil)) == -1 || (fp = fdopen(fd, w)) == NULL) {
-   if (fd != -1)
-   close(fd);
+   if ((fd = mkstemp(tmpfil)) == -1 ||
+   (fp = fdopen(fd, w)) == NULL) {
warn(%s, tmpfil);
+   if (fd != -1) {
+   unlink(tmpfil);
+   close(fd);
+   }
return (1);
}
display(fp, lp, 0, 1);
Index: sbin/scsi/scsi.c
===
RCS file: /cvs/src/sbin/scsi/scsi.c,v
retrieving revision 1.28
diff -u -p -d -r1.28 scsi.c
--- sbin/scsi/scsi.c12 Nov 2013 04:59:02 -  1.28
+++ sbin/scsi/scsi.c11 Jul 2014 16:20:22 -
@@ -571,8 +571,12 @@ edit_init(void)
strlcpy(edit_name, /var/tmp/sc, sizeof edit_name);
if ((fd = mkstemp(edit_name)) == -1)
err(1, mkstemp);
-   if ( (edit_file = fdopen(fd, w+)) == 0)
-   err(1, fdopen);
+   if ( (edit_file = fdopen(fd, w+)) == 0) {
+   int saved_errno = errno;
+   unlink(edit_name);
+   close(fd);
+   errc(1, saved_errno, fdopen);
+   }
edit_opened = 1;
 
atexit(edit_done);
Index: usr.bin/gzsig/sign.c
===
RCS file: /cvs/src/usr.bin/gzsig/sign.c,v
retrieving revision 1.13
diff -u -p -d -r1.13 sign.c
--- usr.bin/gzsig/sign.c10 Mar 2013 10:36:57 -  1.13
+++ usr.bin/gzsig/sign.c11 Jul 2014 16:20:25 -
@@ -281,6 +281,7 @@ sign(int argc, char *argv[])
if ((fout = fdopen(fd, w)) == NULL) {
fprintf(stderr, Error opening %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
close(fd);
continue;
@@ -288,6 +289,7 @@ sign(int argc, char *argv[])
if (copy_permissions(fileno(fin), fd)  0) {
fprintf(stderr, Error initializing %s: %s\n,
tmppath, strerror(errno));
+   unlink(tmppath);
fclose(fin);
fclose(fout);
continue;
Index: usr.bin/htpasswd/htpasswd.c
===
RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
retrieving revision 1.10
diff -u -p -d -r1.10 htpasswd.c
--- usr.bin/htpasswd/htpasswd.c 24 Mar 2014 20:33:01 -  1.10
+++ usr.bin/htpasswd/htpasswd.c 11 Jul 2014 16:20:25 -
@@ -164,8 +164,11 @@ main(int argc, char** argv)
if ((fd = mkstemp(tmpl)) == -1)
err(1, mkstemp);
 
-   if ((out = fdopen(fd, w+)) == NULL)
-   err(1, cannot open tempfile);
+   if

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Marc Espie 
On Fri, Jul 11, 2014 at 04:55:36PM +, Doug Hogan wrote:
 Index: usr.bin/m4/eval.c
 ===
 RCS file: /cvs/src/usr.bin/m4/eval.c,v
 retrieving revision 1.72
 diff -u -p -d -r1.72 eval.c
 --- usr.bin/m4/eval.c 28 Apr 2014 12:34:11 -  1.72
 +++ usr.bin/m4/eval.c 11 Jul 2014 16:20:25 -
 @@ -818,8 +818,12 @@ dodiv(int n)
   char fname[] = _PATH_DIVNAME;
  
   if ((fd = mkstemp(fname))  0 || 
 - (outfile[n] = fdopen(fd, w+)) == NULL)
 - err(1, %s: cannot divert, fname);
 + (outfile[n] = fdopen(fd, w+)) == NULL) {
 + int saved_errno = errno;
 + if (fd != -1)
 + unlink(fname);
 + errc(1, saved_errno, %s: cannot divert, 
 fname);
 + }
   if (unlink(fname) == -1)
   err(1, %s: cannot unlink, fname);
   }
I don't like that part. The logic is a bit wrong. Especially since 
unlink(fname) is always called for fd != -1, so I feel there should be one
single call.

Re: PATCH: misc mkstemp and fdopen fixes

2014-07-11 Thread Doug Hogan 
On Fri, Jul 11, 2014 at 07:29:06PM +0200, Marc Espie wrote:
 I don't like that part. The logic is a bit wrong. Especially since 
 unlink(fname) is always called for fd != -1, so I feel there should be one
 single call.

Ok

Index: usr.bin/m4/eval.c
===
RCS file: /cvs/src/usr.bin/m4/eval.c,v
retrieving revision 1.72
diff -u -p -d -r1.72 eval.c
--- usr.bin/m4/eval.c   28 Apr 2014 12:34:11 -  1.72
+++ usr.bin/m4/eval.c   11 Jul 2014 18:09:31 -
@@ -817,11 +817,10 @@ dodiv(int n)
if (outfile[n] == NULL) {
char fname[] = _PATH_DIVNAME;
 
-   if ((fd = mkstemp(fname))  0 || 
-   (outfile[n] = fdopen(fd, w+)) == NULL)
-   err(1, %s: cannot divert, fname);
-   if (unlink(fname) == -1)
-   err(1, %s: cannot unlink, fname);
+   if ((fd = mkstemp(fname))  0 ||
+   unlink(fname) == -1 ||
+   (outfile[n] = fdopen(fd, w+)) == NULL)
+   err(1, %s: cannot divert, fname);
}
active = outfile[n];
 }