You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Scott Zhong <Sc...@roguewave.com> on 2008/03/10 22:12:37 UTC
STDCXX-401
There are a few files that this affects.
tests/src/file.cpp
src/file.cpp
util/memchk.cpp
tests/src/file.cpp change is listed:
Index: file.cpp
===================================================================
--- file.cpp (revision 634377)
+++ file.cpp (working copy)
@@ -205,8 +205,15 @@
#ifndef _RWSTD_NO_MKSTEMP
# define TMP_TEMPLATE "tmpfile-XXXXXX"
+ char* tmpdir = getenv("TMPDIR");
+
+ if (NULL == tmpdir)
+ {
+ tmpdir = P_tmpdir;
+ }
+
if (!buf) {
- static char fname_buf [sizeof (P_tmpdir) + sizeof
(TMP_TEMPLATE)];
+ static char fname_buf [sizeof (tmpdir) + sizeof
(TMP_TEMPLATE)];
buf = fname_buf;
*buf = '\0';
@@ -214,13 +221,13 @@
if ('\0' == *buf) {
// copy the template to the buffer; make sure there is exactly
- // one path separator character between P_tmpdir and the file
+ // one path separator character between tmpdir and the file
// name template (it doesn't really matter how many there are
// as long as it's at least one, but one looks better than two
// in diagnostic messages)
- size_t len = sizeof (P_tmpdir) - 1;
+ size_t len = sizeof (tmpdir) - 1;
- memcpy (buf, P_tmpdir, len);
+ memcpy (buf, tmpdir, len);
if (_RWSTD_PATH_SEP != buf [len - 1])
buf [len++] = _RWSTD_PATH_SEP;
@@ -248,7 +255,7 @@
# if defined (_WIN32) || defined (_WIN64)
// create a temporary file name
- char* fname = tempnam (P_tmpdir, ".rwtest-tmp");
+ char* fname = tempnam (tmpdir, ".rwtest-tmp");
if (fname) {
@@ -269,7 +276,7 @@
else {
fprintf (stderr, "%s:%d: tempnam(\"%s\", \"%s\") failed: %s\n",
__FILE__, __LINE__,
- P_tmpdir, ".rwtest-tmp", strerror (errno));
+ tmpdir, ".rwtest-tmp", strerror (errno));
}
# else
Re: STDCXX-401
Posted by Martin Sebor <se...@roguewave.com>.
Scott Zhong wrote:
>
>> -----Original Message-----
>> From: Martin Sebor [mailto:msebor@gmail.com] On Behalf Of Martin Sebor
>> Sent: Monday, March 10, 2008 3:39 PM
>> To: dev@stdcxx.apache.org
>> Subject: Re: STDCXX-401
>>
>> Thanks for the patch, Scott! I think this a start, but suspect
>> there are a few other places in the test suite that might need
>> to be changed as well (the locale tests?)
>>
>
> The only function I saw that needed this change was
> rw_tmpnam()(implemented in file.cpp), I believe locale test uses
> rw_tmpnam to create the temporary file.
That may be but there are also makefiles and scripts. Of these,
at least the run_locale_utils.sh shell script uses TMP instead
of TMPDIR. I knew there was a script that did that when I first
responded, I was just too lazy to find it. I found it now but
I wonder if there are others like it. We need to review them
all to make sure they all do the right thing.
>
[...]
>>
>
> Sorry Martin, I follow the guidelines more closely. Since there are 2
> more files with this patch, are subtasks needed?
I don't think so.
My rule of thumb for splitting up an issue into subtasks is when
the issue readily decomposes into one or more clearly separable
tasks that are useful to track separately. STDCXX-401 is about the
whole test suite honoring the TMPDIR variable, so splitting it up
into a subtask for each file that needs to be changes doesn't seem
necessary or appropriate. I don't think there are any dependencies
between the changes to each of the files and I can't think of any
reason why tracking the task of changing of each of the files
separately would be useful.
Here's some background on subtasks from the Jira manual:
http://www.atlassian.com/software/jira/docs/latest/subtasks_creating.html
Martin
RE: STDCXX-401
Posted by Scott Zhong <Sc...@roguewave.com>.
> -----Original Message-----
> From: Martin Sebor [mailto:msebor@gmail.com] On Behalf Of Martin Sebor
> Sent: Monday, March 10, 2008 3:39 PM
> To: dev@stdcxx.apache.org
> Subject: Re: STDCXX-401
>
> Thanks for the patch, Scott! I think this a start, but suspect
> there are a few other places in the test suite that might need
> to be changed as well (the locale tests?)
>
The only function I saw that needed this change was
rw_tmpnam()(implemented in file.cpp), I believe locale test uses
rw_tmpnam to create the temporary file.
> Let me take this opportunity to ask you to please review our
> guidelines for Submitting Patches and the expected Patch Format:
>
> http://stdcxx.apache.org/bugs.html#patches
> http://stdcxx.apache.org/bugs.html#patch_format
>
> Specifically, the string [PATCH] in the subject line is helpful
> in identifying patches among other discussions on the list (and
> in setting up mail filters for such things). Unless the patch is
> entirely trivial, we also need some narrative to go with it. For
> simple patches a Change Log entry usually does the job (see bullet
> 3). For anything even moderately involved, a description of what
> the patch actually does is recommended (bullet 1).
>
> Finally, while we don't have a style document for stdcxx (yet) we
> do follow pretty consistent formatting style. To make integrating
> your patches easier for us please take a look at nearby code and
> try to follow the same style especially WRT braces and whitespace
> usage.
>
> Martin
>
Sorry Martin, I follow the guidelines more closely. Since there are 2
more files with this patch, are subtasks needed?
Re: STDCXX-401
Posted by Martin Sebor <se...@roguewave.com>.
Thanks for the patch, Scott! I think this a start, but suspect
there are a few other places in the test suite that might need
to be changed as well (the locale tests?)
Let me take this opportunity to ask you to please review our
guidelines for Submitting Patches and the expected Patch Format:
http://stdcxx.apache.org/bugs.html#patches
http://stdcxx.apache.org/bugs.html#patch_format
Specifically, the string [PATCH] in the subject line is helpful
in identifying patches among other discussions on the list (and
in setting up mail filters for such things). Unless the patch is
entirely trivial, we also need some narrative to go with it. For
simple patches a Change Log entry usually does the job (see bullet
3). For anything even moderately involved, a description of what
the patch actually does is recommended (bullet 1).
Finally, while we don't have a style document for stdcxx (yet) we
do follow pretty consistent formatting style. To make integrating
your patches easier for us please take a look at nearby code and
try to follow the same style especially WRT braces and whitespace
usage.
Martin
Scott Zhong wrote:
> There are a few files that this affects.
>
> tests/src/file.cpp
> src/file.cpp
> util/memchk.cpp
>
> tests/src/file.cpp change is listed:
>
> Index: file.cpp
> ===================================================================
> --- file.cpp (revision 634377)
> +++ file.cpp (working copy)
> @@ -205,8 +205,15 @@
> #ifndef _RWSTD_NO_MKSTEMP
> # define TMP_TEMPLATE "tmpfile-XXXXXX"
>
> + char* tmpdir = getenv("TMPDIR");
> +
> + if (NULL == tmpdir)
> + {
> + tmpdir = P_tmpdir;
> + }
> +
> if (!buf) {
> - static char fname_buf [sizeof (P_tmpdir) + sizeof
> (TMP_TEMPLATE)];
> + static char fname_buf [sizeof (tmpdir) + sizeof
> (TMP_TEMPLATE)];
>
> buf = fname_buf;
> *buf = '\0';
> @@ -214,13 +221,13 @@
>
> if ('\0' == *buf) {
> // copy the template to the buffer; make sure there is exactly
> - // one path separator character between P_tmpdir and the file
> + // one path separator character between tmpdir and the file
> // name template (it doesn't really matter how many there are
> // as long as it's at least one, but one looks better than two
> // in diagnostic messages)
> - size_t len = sizeof (P_tmpdir) - 1;
> + size_t len = sizeof (tmpdir) - 1;
>
> - memcpy (buf, P_tmpdir, len);
> + memcpy (buf, tmpdir, len);
> if (_RWSTD_PATH_SEP != buf [len - 1])
> buf [len++] = _RWSTD_PATH_SEP;
>
> @@ -248,7 +255,7 @@
> # if defined (_WIN32) || defined (_WIN64)
>
> // create a temporary file name
> - char* fname = tempnam (P_tmpdir, ".rwtest-tmp");
> + char* fname = tempnam (tmpdir, ".rwtest-tmp");
>
> if (fname) {
>
> @@ -269,7 +276,7 @@
> else {
> fprintf (stderr, "%s:%d: tempnam(\"%s\", \"%s\") failed: %s\n",
> __FILE__, __LINE__,
> - P_tmpdir, ".rwtest-tmp", strerror (errno));
> + tmpdir, ".rwtest-tmp", strerror (errno));
> }
>
> # else
>