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 814acb12169ab36455638377fc904ec2868b9d1a..be0cd3f467fbdef0cf00128785f9f1c404b3d567 100644 (file)
--- 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 4e7b76a02e62098c096a9bb201230af31e720e18..1f6117cf72056e43a3d8ce48813525d3c2690998 100644 (file)
--- 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 0e01206f6890db3312ad1dc1a5f32f4238459b33..71f454c9912b0e8b1ec2589b5de268cedcf573de 100644 (file)
--- 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 f26867f68aa9a87b83971f3b5e09e462bec53a9d..c61f19de134c7a9ffc4a5e1a9a87a886507b8ca4 100644 (file)
--- 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 cec1012f3521d0a8d9ed3c29645091c0e347f827..e59b3e03c55c4b46b2c59c705c382996a8db11b4 100644 (file)
--- 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 );