Re: [PATCH] use mkstemp instead of mktemp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

> >  	FILE *mnttmp, *mntmtab;
> >  	struct mntent *mountent;
> >  	char *mtabfile, *mtabdir, *mtabtmpfile;
> > +	mode_t mode;
> > +	FILE *spf;
> > +	int fd = -1 ;
> >  
> >  	mtabfile = strdup(MOUNTED);
> >  	mtabdir = dirname(mtabfile);
> > @@ -1652,12 +1655,17 @@ del_mtab(char *mountpoint)
> >  		goto del_mtab_exit;
> >  	}
> >  
> > -	mtabtmpfile = mktemp(mtabtmpfile);
> > -	if (!mtabtmpfile) {
> > -		fprintf(stderr, "del_mtab: cannot setup tmp file destination");
> > -		rc = EX_FILEIO;
> > -		goto del_mtab_exit;
> > +	mode = umask(0077);
> 
> (cc'ing Carlos since he did most of this work originally)
> 
> I think we want the final mtab to be group and world-readable, right?
> So maybe umask(0033) would be better? We should probably also reset the
> umask afterward too just to be safe...
> 
> > +	(void) umask(mode);
> > +	if ((fd = mkstemp(mtabtmpfile)) == -1 ||
> > +		(spf = fdopen(fd, "w+")) == NULL) {
> > +		if (fd != -1) {
> > +			fprintf(stderr, "del_mtab: cannot setup tmp file destination");
> > +			rc = EX_FILEIO;
> > +			goto del_mtab_exit;
> > +                }
> >  	}
> > +	(void) fclose(spf);
> >  
> 
> Looks reasonable to change to mkstemp, but what's the point of doing the
> the fdopen there if you're just going to immediately close it
> afterward? Why not just close(fd) there?
> 
The mainly reason I chose mktemp() instead of mkstemp was due its return value.
The mktemp() returns a template, while mkstemp returns a file descriptor, which
makes the need to handle the file descriptor.
Due our usage of mktemp() here, I didn't think we were subjected to both risks
it implies (the limit of at most 26 different names and the race), and also,
mktemp() made it easier to implement. But I have no objection to change it to
mkstemp if anyone believes it's a risk.

 
Sorry for my delay though, I was enjoying some vacations :)

-- 
--Carlos
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux