From: Florian Krohm Date: Sun, 9 Nov 2014 16:15:23 +0000 (+0000) Subject: Change VG_(mkstemp) such that X-Git-Tag: svn/VALGRIND_3_11_0~842 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ab2599f4cff93dcf3bda31ca0b47746f43f24cc2;p=thirdparty%2Fvalgrind.git Change VG_(mkstemp) such that (a) the 2nd argument must not be NULL This was true anyhow and requiring it allows us to simplify the function by eliminating the local buffer. (b) the memory pointed to by the 2nd argument is always initialised In the past the output file name was not initialised in case VG_(open) failed 10 times in a row. The call sites in m_main.c and m_gdbserver/target.c were reading the uninitialised filename unconditionally. This was spotted by IBM's BEAM checker. Fix call sites, eliminate some magic constants along the way. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14706 --- diff --git a/coregrind/m_debuginfo/readpdb.c b/coregrind/m_debuginfo/readpdb.c index 814acb1216..be0cd3f467 100644 --- a/coregrind/m_debuginfo/readpdb.c +++ b/coregrind/m_debuginfo/readpdb.c @@ -2415,7 +2415,8 @@ HChar* ML_(find_name_of_pdb_file)( const HChar* pename ) /* This is a giant kludge, of the kind "you did WTF?!?", but it works. */ Bool do_cleanup = False; - HChar tmpname[VG_(mkstemp_fullname_bufsz)(50-1)], tmpnameroot[50]; + HChar tmpnameroot[50]; // large enough + HChar tmpname[VG_(mkstemp_fullname_bufsz)(sizeof tmpnameroot - 1)]; Int fd, r; HChar* res = NULL; @@ -2429,7 +2430,7 @@ HChar* ML_(find_name_of_pdb_file)( const HChar* pename ) fd = VG_(mkstemp)( tmpnameroot, tmpname ); if (fd == -1) { VG_(message)(Vg_UserMsg, - "Find PDB file: Can't create /tmp file %s\n", tmpname); + "Find PDB file: Can't create temporary file %s\n", tmpname); goto out; } do_cleanup = True; diff --git a/coregrind/m_gdbserver/target.c b/coregrind/m_gdbserver/target.c index 4e7b76a02e..1f6117cf72 100644 --- a/coregrind/m_gdbserver/target.c +++ b/coregrind/m_gdbserver/target.c @@ -558,11 +558,14 @@ static Bool getplatformoffset (SizeT *result) // lm_modid_offset is a magic offset, retrieved using an external program. if (!getplatformoffset_called) { + getplatformoffset_called = True; const HChar *platform = VG_PLATFORM; const HChar *cmdformat = "%s/%s-%s -o %s"; const HChar *getoff = "getoff"; HChar outfile[VG_(mkstemp_fullname_bufsz) (VG_(strlen)(getoff))]; Int fd = VG_(mkstemp) (getoff, outfile); + if (fd == -1) + return False; HChar cmd[ VG_(strlen)(cmdformat) + VG_(strlen)(VG_(libdir)) - 2 + VG_(strlen)(getoff) - 2 @@ -617,7 +620,6 @@ static Bool getplatformoffset (SizeT *result) ret = VG_(unlink)( outfile ); if (ret != 0) VG_(umsg) ("error: could not unlink %s\n", outfile); - getplatformoffset_called = True; } *result = lm_modid_offset; diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c index 0e01206f68..71f454c991 100644 --- a/coregrind/m_libcfile.c +++ b/coregrind/m_libcfile.c @@ -760,13 +760,13 @@ SizeT VG_(mkstemp_fullname_bufsz) ( SizeT part_of_name_len ) Int VG_(mkstemp) ( const HChar* part_of_name, /*OUT*/HChar* fullname ) { - HChar buf[VG_(mkstemp_fullname_bufsz)(VG_(strlen)(part_of_name))]; - Int n, tries, fd; + Int n, tries; UInt seed; SysRes sres; const HChar *tmpdir; vg_assert(part_of_name); + vg_assert(fullname); n = VG_(strlen)(part_of_name); vg_assert(n > 0 && n < 100); @@ -779,23 +779,20 @@ Int VG_(mkstemp) ( const HChar* part_of_name, /*OUT*/HChar* fullname ) while (True) { if (tries++ > 10) return -1; - VG_(sprintf)( buf, mkstemp_format, + VG_(sprintf)( fullname, mkstemp_format, tmpdir, part_of_name, VG_(random)( &seed )); if (0) - VG_(printf)("VG_(mkstemp): trying: %s\n", buf); + VG_(printf)("VG_(mkstemp): trying: %s\n", fullname); - sres = VG_(open)(buf, + sres = VG_(open)(fullname, VKI_O_CREAT|VKI_O_RDWR|VKI_O_EXCL|VKI_O_TRUNC, VKI_S_IRUSR|VKI_S_IWUSR); if (sr_isError(sres)) { - VG_(umsg)("VG_(mkstemp): failed to create temp file: %s\n", buf); + VG_(umsg)("VG_(mkstemp): failed to create temp file: %s\n", fullname); continue; } /* VG_(safe_fd) doesn't return if it fails. */ - fd = VG_(safe_fd)( sr_Res(sres) ); - if (fullname) - VG_(strcpy)( fullname, buf ); - return fd; + return VG_(safe_fd)( sr_Res(sres) ); } /* NOTREACHED */ } diff --git a/coregrind/m_main.c b/coregrind/m_main.c index f26867f68a..c61f19de13 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -1893,7 +1893,8 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp ) VG_(cl_auxv_fd) = -1; #else if (!need_help) { - HChar buf[50], buf2[VG_(mkstemp_fullname_bufsz)(50-1)]; + HChar buf[50]; // large enough + HChar buf2[VG_(mkstemp_fullname_bufsz)(sizeof buf - 1)]; HChar nul[1]; Int fd, r; const HChar* exename; diff --git a/coregrind/pub_core_libcfile.h b/coregrind/pub_core_libcfile.h index cec1012f35..e59b3e03c5 100644 --- a/coregrind/pub_core_libcfile.h +++ b/coregrind/pub_core_libcfile.h @@ -95,8 +95,8 @@ extern SysRes VG_(pread) ( Int fd, void* buf, Int count, OffT offset ); extern SizeT VG_(mkstemp_fullname_bufsz) ( SizeT part_of_name_len ); /* Create and open (-rw------) a tmp file name incorporating said arg. - Returns -1 on failure, else the fd of the file. If fullname is - non-NULL, the file's name is written into it. The number of bytes written + Returns -1 on failure, else the fd of the file. The file name is + written to the memory pointed to be fullname. The number of bytes written is equal to VG_(mkstemp_fullname_bufsz)(VG_(strlen)(part_of_name)). */ extern Int VG_(mkstemp) ( const HChar* part_of_name, /*OUT*/HChar* fullname );