On Tue, 2011-07-05 at 14:14 -0700, Khem Raj wrote: > On Mon, Jul 4, 2011 at 2:39 AM, Phil Blundell <[email protected]> wrote: > > On Sat, 2011-07-02 at 17:47 -0700, Khem Raj wrote: > >> +- if ((fd = mkostemp(temp, O_CLOEXEC|O_CREAT|O_WRONLY)) < 0) { > >> ++ if ((fd = mkstemp(temp)) < 0) { > > > > This change looks like it will effectively remove O_CLOEXEC without > > reinstating it anywhere else. Have you verified this is a safe thing to > > do? > > Initial tests on uclibc seems to work well. but this needs more testing > thats why the patch is only applied for uclibc based systems
Just to reiterate what I mentioned on IRC the other day, I don't think empirical testing is the right way to address this problem, at least not in isolation. Absent an exceptionally comprehensive testsuite, there is too much risk that some kind of failure would be overlooked. I think code inspection is required here to establish whether O_CLOEXEC is needed and what to do about it. The easy answer to the latter is probably just to add an appropriate fcntl() call. > >> ++ char dev[20]; > >> + int prio = 0, k; > >> + > >> + if ((k = fscanf(m->proc_swaps, > >> +- "%ms " /* device/file */ > >> ++ "%19s " /* device/file */ > >> + "%*s " /* type of swap */ > > > > Is 19 characters definitely enough? If this is potentially a pathname > > then it could be much longer than that. > > > > it just reads /dev/* so should be ok. I haven't checked the actual code, but the implication from the variable names and comments shown above is that it's reading /proc/swaps. If that's the case then, in the situation where you're swapping to a file, the pathnames involved could be arbitrarily long. p. _______________________________________________ Openembedded-devel mailing list [email protected] http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
