> -----Original Message-----
> From: Garrett Cooper [mailto:[email protected]]
> Sent: Saturday, February 13, 2010 4:19 AM
> To: Mitani
> Cc: [email protected]
> Subject: Re: [LTP] "stime01" problem
>
> On Fri, Feb 5, 2010 at 8:22 AM, Garrett Cooper <[email protected]>
> wrote:
> > On Fri, Feb 5, 2010 at 8:19 AM, Garrett Cooper <[email protected]>
> wrote:
> >> On Thu, Feb 4, 2010 at 10:27 PM, Mitani <[email protected]> wrote:
> >>> Hi,
> >>>
> >>> I noticed that "stime01" test let system-clock be faster than right
> time.
> >>>
> >>> "stime01.c" is going to set system-clock by calling "stime()"
> function
> >>> with "new_time" variable.
> >>> It gets "curr_time" by "time()" function, and sets "curr_time +
> 10sec"
> >>> in "new_time".
> >>> But after the test, It doesn't restore system-clock.
> >>>
> >>> Therefore, system-clock is set faster than right time about 10
> seconds
> >>> after "stime01" test.
> >>> If this test is repeated many times, the system-clock advances for
> the
> >>> number of test times.
> >>> And, if system is rebooted, hardware-clock will be wrong.
> >>>
> >>> How about following patch?
> >>
> >> The patch looks good, but I assume (and hope) that cleanup gets
> >> executed every time before the program exits?
> >
> > Err... two other problems with the above code now that I look at it
> a
> > bit closer...
> >
> > 1. tst_brkm in cleanup shouldn't have cleanup as the second arg; I'm
> > not sure whether or not tst_brkm handles infinite recursion properly;
> > NULL should be the second arg (if you don't want to exit) or tst_exit
> > (if you do want to exit).
> > 2. What happens if you try to restore the time and it failed to get
> > the original time (shouldn't happen, but it can happen if the RTC
> > driver is busted, like it was on the cavium image that I was using
> at
> > my previous job)?
>
> Could you please submit a modified patch?
> Thanks!
> -Garrett
I am sorry for the delay in my response.
---
> The patch looks good, but I assume (and hope) that cleanup gets
>executed every time before the program exits?
I added process for restore system-time in "cleanup()" function.
As you say, in this measure, it is a premise that cleanup() is called
when a program exits.
"cleanup()" should be called when the program exits after system-time
was changed by "stime()".
When the error occurred after "stime()" called, the program calls
"tst_brkm()" or "txt_resm()".
(1) There is one "tst_brkm()", and it calls "cleanup()" by its second
argument.
(2) There are several "tst_resm()", and they don't call exit().
Therefore, cleanup() is called at the last of "main()" function.
Due to the above, I think that "cleanup()" will be called in all cases
and it isn't neccessary to revise them.
---
>1. tst_brkm in cleanup shouldn't have cleanup as the second arg; I'm
>not sure whether or not tst_brkm handles infinite recursion properly;
>NULL should be the second arg (if you don't want to exit) or tst_exit
>(if you do want to exit).
You are right. I think my previous revision is obviously mistake.
I judge that "tst_brkm()" function doesn't have to exit because
"cleanup()" function calls "tst_exit()".
I think "NULL" should be the second argument of "tst_brkm()".
---
>2. What happens if you try to restore the time and it failed to get
>the original time (shouldn't happen, but it can happen if the RTC
>driver is busted, like it was on the cavium image that I was using at
>my previous job)?
I couldn't understand meanings of the sentence "it failed to get the
original time" well. Do you mean "gettimeofday()" which I added in
"setup()" function?
If you mean "gettimeofday()", this test will failed or abnormal time
will be set when "gettimeofday()" got abnormal time.
(1) The case of test failure:
In my first revision, the test will exit by "tst_brkm()" function.
But, if cleanup() is called in this "tst_brkm()", "settimeofday()"
will be called with the abnormal time.
Therefore, calling "cleanup()" is wrong.
I set "NULL" to the second argument of "tst_brkm()", and call
"tst_exit()" instead of it.
"time()" just above is same case. I apply the same measures to
the case of "time()" failure.
(2) The case of abnormal time setting:
"time()" which called by "setup()" function also calls
"gettimeofday()" internally.
"stime()" -- which is this test's purpose -- sets the time (added 10
seconds at the result of "time()") in system-time.
Therefore, this test itself will set abnormal time to system-time
if "gettimeofday()" performed abnormally.
The risk with gettimeofday() is the same as the risk which this test
itself has, I think.
By another way of thinking, if system-time was set 10 seconds faster,
it is a range of tolerance and there isn't the troubled person so
much.
If The risk of revision is big, I think that there isn't so the
meaning to correct daringly.
The judgment whether or not I revise it entrusts you.
---
> Could you please submit a modified patch?
Here the fixed patch.
============
--- ./testcases/kernel/syscalls/stime/stime01.c 2009-03-23
22:36:07.000000000 +0900
+++ ./testcases/kernel/syscalls/stime/stime01.c.new 2010-02-22
13:22:25.000000000 +0900
@@ -70,6 +70,7 @@
#include <sys/types.h>
#include <errno.h>
#include <fcntl.h>
+#include <sys/time.h>
#include <time.h>
#include <string.h>
#include <sys/stat.h>
@@ -83,6 +84,7 @@
char *TCID = "stime01"; /* Test program identifier. */
int TST_TOTAL = 1; /* Total number of test cases. */
extern int Tst_count; /* Test Case counter for tst_* routines */
+struct timeval first_tv; /* system's current time in micro-seconds */
time_t curr_time; /* system's current time in seconds */
time_t new_time; /* system's new time */
time_t tloc; /* argument var. for time() */
@@ -187,12 +189,20 @@
/* Get the current time */
if ((curr_time = time(&tloc)) < 0) {
- tst_brkm(TBROK, cleanup,
+ tst_brkm(TBROK, NULL,
"time() failed to get current time, errno=%d",
errno);
+ tst_exit();
/*NOTREACHED*/}
/* Get the system's new time */
new_time = curr_time + INCR_TIME;
+
+ /* Get the current time in microsecond */
+ if (gettimeofday(&first_tv, NULL) != 0) {
+ tst_brkm(TBROK, NULL,
+ "gettimeofday() failed to get current time,
errno=%d", errno);
+ tst_exit();
+ }
} /* End setup() */
/*
@@ -202,6 +212,10 @@
*/
void cleanup()
{
+ /* restore current time */
+ if (settimeofday(&first_tv, NULL) != 0)
+ tst_brkm(TBROK, NULL,
+ "settimeofday() failed to restore current time,
errno=%d", errno);
/*
* print timing stats if that option was specified.
* print errno log if that option was specified.
============
Thank you,
-Tomonori Mitani
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
