On Wed, Jan 02, 2002 at 08:22:20PM +0100, zoppiat_private wrote: > Hi, i've implemented this function as part of my bondlog package: Where is the documentation? (i.e.: how about comments?) > #include <string.h> > #include <ctype.h> > #include <stdio.h> > #include <stdarg.h> > #include <stdlib.h> > #include <errno.h> > #include "bondlog.h" > #include <time.h> > #include <sys/types.h> > #include <sys/stat.h> > #include <fcntl.h> > #include <unistd.h> > #include <pwd.h> > > > #define PATH_MAX 1024 This is system dependend and should be taken from local .h file. > FILE * > mytmpfile () > { > > char appname[] = "/var/spool/bondlog/tmp-"; > char filename[PATH_MAX]; > base64buf b; > unsigned int num = 0; > unsigned int count = 0; > mode_t mask; > FILE *urand_file; > FILE *tmp; > unsigned char *stream; > > if ((stream = (char *) malloc (11)) == NULL) ^^^^ (see below) > { > return NULL; > } > memset (stream, '\0', 11); ^^ This relies on one remembering to change BOTH values. make a #define > you want to *create* a file. Why are you trying to open it without any checks done before? > urand_file = fopen (RANDOMFILE, "r"); > > if (urand_file == NULL) > { > free (stream); > return NULL; > } You are returning NULL if the file does NOT exist. This seems to be the opposite of what you want to. (I guess, cause there are no comments) > while (count < 9) > { > fread (&num, sizeof (unsigned int), 1, urand_file); > stream[count] = (char) num; > count++; > } > fclose (urand_file); Oh, now I can guess the urand_file may be something like /dev/random or so. Perhaps this would be nice to read from a comment. > base64_encode (&b, stream, 9); > memset (filename, '\0', sizeof (filename)); > snprintf (filename, sizeof (filename), "%s%s", appname, b.data); you may write one byte LESS than your array has bytes (trailing \0), you defined PATH_MAX to 1024 so filename may be 1023 chars maximum. > mask = umask (0066); no executables -> 0177 > if ((tmp = fopen (filename, "wb")) == NULL) > { > fprintf (stderr, "Cannot create filename:%s", filename); > free (stream); > return NULL; > } race condition: make sure, your process is the only one to create this file (open with O_CREAT and O_EXCL). symlink trouble: use lstat() before opening. use stat() to check if you are the one to open the file. > unlink (filename); // the file will be not delete until closed; > umask (mask); > free (stream); > buffer_delete (&b); > return (tmp); Question: why do you use malloc and local variables in the way you do? > Could be function this considered a valid safe tempfile alternative ? just think about it again. This may not be the whole thing. I just took a glimpse. -- Christian Recktenwald : : citecs GmbH : chrisat_private : Unternehmensberatung fuer : voice +49 711 601 2090 : Burgstallstrasse 54 EDV und Telekommunikation : fax +49 711 601 2092 : D-70199 Stuttgart
This archive was generated by hypermail 2b30 : Thu Jan 03 2002 - 10:12:48 PST