You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2016/02/03 15:27:01 UTC
svn commit: r1728324 - /subversion/trunk/subversion/tests/svn_test_main.c
Author: philip
Date: Wed Feb 3 14:27:01 2016
New Revision: 1728324
URL: http://svn.apache.org/viewvc?rev=1728324&view=rev
Log:
* subversion/tests/svn_test_main.c
(call_setjmp): New, to fix some potential setjmp/longjmp clobbers.
(do_test_num): Call new function, tweak comment to explain why
a parameter is modified.
Modified:
subversion/trunk/subversion/tests/svn_test_main.c
Modified: subversion/trunk/subversion/tests/svn_test_main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/svn_test_main.c?rev=1728324&r1=1728323&r2=1728324&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/svn_test_main.c (original)
+++ subversion/trunk/subversion/tests/svn_test_main.c Wed Feb 3 14:27:01 2016
@@ -393,6 +393,16 @@ log_results(const char *progname,
return test_failed;
}
+/* This function exists so that automatic variables in the calling
+ function are peserved. At the time of writing 'err' and 'test_num'
+ in 'do_test_num()' were in danger of being clobbered by a direct
+ setjmp() call. */
+static int call_setjmp(jmp_buf env)
+{
+ return setjmp(env);
+}
+
+
/* Execute a test number TEST_NUM. Pretty-print test name and dots
according to our test-suite spec, and return the result code.
If HEADER_MSG and *HEADER_MSG are not NULL, print *HEADER_MSG prior
@@ -414,9 +424,11 @@ do_test_num(const char *progname,
svn_boolean_t run_this_test; /* This test's mode matches DESC->MODE. */
enum svn_test_mode_t test_mode;
- /* Check our array bounds! */
+ /* This allows './some-test -- -1' to run the last test. */
if (test_num < 0)
test_num += array_size + 1;
+
+ /* Check our array bounds! */
if ((test_num > array_size) || (test_num <= 0))
{
if (header_msg && *header_msg)
@@ -458,7 +470,7 @@ do_test_num(const char *progname,
so we don't end up in an infinite loop.
If we've got non-zero from setjmp(), we know we've crashed. */
- if (setjmp(jump_buffer) == 0)
+ if (call_setjmp(jump_buffer) == 0)
{
/* Do test */
if (msg_only || skip || !run_this_test)
Re: svn commit: r1728324 - /subversion/trunk/subversion/tests/svn_test_main.c
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> On Thu, Feb 04, 2016 at 12:38:54PM +0100, Bert Huijben wrote:
>> I don't think this is allowed.
Ah yes, 7.13.2.1/2 agrees.
> Isn't the right fix to mark the affected variables as volatile?
It is slightly more involved because one variable is a parameter,
1728464.
--
Philip Martin
WANdisco
Re: svn commit: r1728324 -
/subversion/trunk/subversion/tests/svn_test_main.c
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Feb 04, 2016 at 12:38:54PM +0100, Bert Huijben wrote:
>
>
> > -----Original Message-----
> > From: philip@apache.org [mailto:philip@apache.org]
> > Sent: woensdag 3 februari 2016 15:27
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1728324 -
> > /subversion/trunk/subversion/tests/svn_test_main.c
> >
> > Author: philip
> > Date: Wed Feb 3 14:27:01 2016
> > New Revision: 1728324
> >
> > URL: http://svn.apache.org/viewvc?rev=1728324&view=rev
> > Log:
> > * subversion/tests/svn_test_main.c
> > (call_setjmp): New, to fix some potential setjmp/longjmp clobbers.
> > (do_test_num): Call new function, tweak comment to explain why
> > a parameter is modified.
> >
> > Modified:
> > subversion/trunk/subversion/tests/svn_test_main.c
> >
> > Modified: subversion/trunk/subversion/tests/svn_test_main.c
> > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/svn_test
> > _main.c?rev=1728324&r1=1728323&r2=1728324&view=diff
> > ==========================================================
> > ====================
> > --- subversion/trunk/subversion/tests/svn_test_main.c (original)
> > +++ subversion/trunk/subversion/tests/svn_test_main.c Wed Feb 3
> > 14:27:01 2016
> > @@ -393,6 +393,16 @@ log_results(const char *progname,
> > return test_failed;
> > }
> >
> > +/* This function exists so that automatic variables in the calling
> > + function are peserved. At the time of writing 'err' and 'test_num'
> > + in 'do_test_num()' were in danger of being clobbered by a direct
> > + setjmp() call. */
> > +static int call_setjmp(jmp_buf env)
> > +{
> > + return setjmp(env);
>
> I don't think this is allowed.
>
> Setjmp is not really a function that you can just call from an inner function. As a macro it stores the current stack state, to allow a later return by resetting the stack to the old state. With an inner function this is not guaranteed to work. (I'm guessing many compilers would just inline the single call, but then...)
>
> Bert
>
>
Isn't the right fix to mark the affected variables as volatile?
RE: svn commit: r1728324 - /subversion/trunk/subversion/tests/svn_test_main.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: philip@apache.org [mailto:philip@apache.org]
> Sent: woensdag 3 februari 2016 15:27
> To: commits@subversion.apache.org
> Subject: svn commit: r1728324 -
> /subversion/trunk/subversion/tests/svn_test_main.c
>
> Author: philip
> Date: Wed Feb 3 14:27:01 2016
> New Revision: 1728324
>
> URL: http://svn.apache.org/viewvc?rev=1728324&view=rev
> Log:
> * subversion/tests/svn_test_main.c
> (call_setjmp): New, to fix some potential setjmp/longjmp clobbers.
> (do_test_num): Call new function, tweak comment to explain why
> a parameter is modified.
>
> Modified:
> subversion/trunk/subversion/tests/svn_test_main.c
>
> Modified: subversion/trunk/subversion/tests/svn_test_main.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/svn_test
> _main.c?rev=1728324&r1=1728323&r2=1728324&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/tests/svn_test_main.c (original)
> +++ subversion/trunk/subversion/tests/svn_test_main.c Wed Feb 3
> 14:27:01 2016
> @@ -393,6 +393,16 @@ log_results(const char *progname,
> return test_failed;
> }
>
> +/* This function exists so that automatic variables in the calling
> + function are peserved. At the time of writing 'err' and 'test_num'
> + in 'do_test_num()' were in danger of being clobbered by a direct
> + setjmp() call. */
> +static int call_setjmp(jmp_buf env)
> +{
> + return setjmp(env);
I don't think this is allowed.
Setjmp is not really a function that you can just call from an inner function. As a macro it stores the current stack state, to allow a later return by resetting the stack to the old state. With an inner function this is not guaranteed to work. (I'm guessing many compilers would just inline the single call, but then...)
Bert