On Sun, Feb 21, 2010 at 11:19 PM, Garrett Cooper <[email protected]> wrote: > Here are my two concerns... > > On Sun, Feb 21, 2010 at 9:53 PM, Mitani <[email protected]> wrote: >>> -----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. > > You're making an assumption that the program will execute to > completion. What if the program is interrupted, or for all intensive > purposes doesn't get to cleanup? The problem that you originally > described still exists, as a horse of a different color, and needs to > be fixed. > >> --- >>>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()". > > If NULL is used, then the tests will be marked broken, but the > application will continue executing; probably not what you want... > >> --- >>>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. > > gettimeofday can and will fail under certain conditions; from gettimeofday(2): > > RETURN VALUE > gettimeofday() and settimeofday() return 0 for success, or -1 > for failure (in which case errno is set appropriately). > > ERRORS > EFAULT One of tv or tz pointed outside the accessible address space. > > EINVAL Timezone (or something else) is invalid. > > EPERM The calling process has insufficient privilege to call > settimeofday(); under Linux the CAP_SYS_TIME capability is required. > > Assuming that a function call will always pass when it can distinguish > pass from failure is a common mistake that gets made in software > engineering in general, and it's something that we (I am by no means > exempt from this) need to avoid. > >> (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.
After tweaking around with this test, I noticed some `funkiness' with tst_sig, so I yanked it, and replaced the time calls with gettimeofday, and voila... it works under error conditions (minus when a signal is sent to it -- that seemed to be a problem...). Watched the clock before and after the test was run and it was happily ticking away with the correct time. Cheers, -Garrett ------------------------------------------------------------------------------ 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
