Bug 16924

You need to do error checking on your calls to malloc.  Other than that, I
think it looks ok on an initial reading.  (I'm certainly not an expert on
libc though.)

Daniel

On Wed, Apr 05, 2000 at 09:54:37PM -0400, Daniel Hagan wrote:
> You need to do error checking on your calls to malloc.  Other than that, I
> think it looks ok on an initial reading.  (I'm certainly not an expert on
> libc though.)

Thanks for looking at this. I wanted to include error checking, but I
wasn't sure of what action to take if an error occurred. I remember
finding some other malloc calls in libc that didn't check for errors, so
I just left it alone. If anyone can point me to an example of how to
handle this, I'd appreciate it.

On Thu, 06 Apr 2000 09:28:55 -0400, Mike Heffner wrote:

> I assume you mean the part of malloc setting ENOMEM. Is there a case
> where it wouldn't set ENOMEM? According to the manpage, this is how
> malloc(3) should behave.

Bleh, please ignore that.  I don't know what I've been smoking, but I'm
actually the guy who modified the malloc(3) manual page to specifically
state that it _does_ set errno! :-)

The problem is that the C standard doesn't specify that malloc() sets
errno.  We should probably mention in malloc(3)'s STANDARDS section that
setting errno from malloc() is mandated by the standard and that other
implementations can not be relied on to do so.

Ciao,
Sheldon.

On Thu, 6 Apr 2000, Lachlan O'Dea wrote:

> Thanks for looking at this. I wanted to include error checking, but I
> wasn't sure of what action to take if an error occurred. I remember
> finding some other malloc calls in libc that didn't check for errors, so
> I just left it alone. If anyone can point me to an example of how to
> handle this, I'd appreciate it.

From the information given in the man page for tmpfile(3), it would appear
that the correct way to handle a malloc failure would be to return null
and leave errno set to whatever value malloc(3) put there.  See the RETURN
VALUES and ERRORS section of the tmpfile(3) manpage.

The original code is incorrect anyway since it doesn't handle malloc
errors.

Daniel

-- 
Daniel Hagan                                             Computer Science CSE
dhagan@cs.vt.edu                                http://www.cs.vt.edu/~dhagan/

On Thu, Apr 06, 2000 at 09:56:33AM -0400, Daniel Hagan wrote:

> The original code is incorrect anyway since it doesn't handle malloc
> errors.

So based on the discussion, all I have to do is return NULL if malloc
returns NULL. The attached diff does this. I also added a paragraph to
the man page.

I didn't add anything to the ERRORS section, as it states that errno can
be set to any of the errors returned by fdopen(3), which in turn can set
errno to any value specified by malloc(3).

Index: tmpfile.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdio/tmpfile.c,v
retrieving revision 1.4
diff -u -r1.4 tmpfile.c
--- tmpfile.c	2000/01/27 23:06:46	1.4
+++ tmpfile.c	2000/04/10 10:34:49
@@ -47,6 +47,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <paths.h>
+#include <stdlib.h>
 
 FILE *
 tmpfile()
@@ -55,10 +56,28 @@
 	FILE *fp;
 	int fd, sverrno;
 #define	TRAILER	"tmp.XXXXXX"
-	char buf[sizeof(_PATH_TMP) + sizeof(TRAILER)];
+	char *buf;
+	char *envtmpdir;
+	int envtmpdirlen;
 
-	(void)memcpy(buf, _PATH_TMP, sizeof(_PATH_TMP) - 1);
-	(void)memcpy(buf + sizeof(_PATH_TMP) - 1, TRAILER, sizeof(TRAILER));
+	if ((envtmpdir = getenv("TMPDIR")) != NULL)
+	{
+		envtmpdirlen = strlen(envtmpdir);
+		buf = malloc(envtmpdirlen + 1 + sizeof(TRAILER));
+		if (buf == NULL)
+			return NULL;
+		(void)memcpy(buf, envtmpdir, envtmpdirlen);
+		buf[envtmpdirlen] = '/';
+		(void)memcpy(buf + envtmpdirlen + 1, TRAILER, sizeof(TRAILER));
+	}
+	else
+	{
+		buf = malloc(sizeof(_PATH_TMP) + sizeof(TRAILER) - 1);
+		if (buf == NULL)
+			return NULL;
+		(void)memcpy(buf, _PATH_TMP, sizeof(_PATH_TMP) - 1);
+		(void)memcpy(buf + sizeof(_PATH_TMP) - 1, TRAILER, sizeof(TRAILER));
+	}
 
 	sigfillset(&set);
 	(void)sigprocmask(SIG_BLOCK, &set, &oset);
@@ -66,6 +85,8 @@
 	fd = mkstemp(buf);
 	if (fd != -1)
 		(void)unlink(buf);
+
+	free(buf);
 
 	(void)sigprocmask(SIG_SETMASK, &oset, NULL);
 
Index: tmpnam.3
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdio/tmpnam.3,v
retrieving revision 1.5
diff -u -r1.5 tmpnam.3
--- tmpnam.3	2000/01/09 08:54:03	1.5
+++ tmpnam.3	2000/04/10 10:34:50
@@ -66,6 +66,16 @@
 The file is opened with the access value
 .Ql w+ .
 .Pp
+The file is initially created in the directory specified by the environment
+variable
+.Ev TMPDIR
+(if set), or
+.Pa /tmp
+by default.
+As the file is immediately unlinked, the only effect of
+.Ev TMPDIR
+is to specify which partition the file is created on.
+.Pp
 The
 .Fn tmpnam
 function

Comment 6 Mike Heffner 2001-07-07 05:09:45 UTC

State Changed
From-To: open->analyzed

Comitted to current, MFC in 2 weeks. 

Comment 7 Mike Heffner 2001-07-07 05:09:45 UTC

Responsible Changed
From-To: freebsd-bugs->	 mikeh

Might as well since I committed the fix ;)

Comment 8 Mike Heffner 2001-08-16 17:24:27 UTC

State Changed
From-To: analyzed->closed

Fix MFC'd.