From: Henry Y. <hy...@mv...> - 2010-05-04 03:15:06
|
Garrett, This patch installs the trap for cleanup_test which removes the sudoers file only if it was installed by utimensat script, and also takes care of detecting whether sudo knows about the -n option. Signed-Off-By: <hy...@mv...> The patch was generated from ltp-dev git tree. I didn't realize Caspar's patch hadn't already been incorporated yet, so didn't account for it. Henry Yei <hy...@mv...> On Fri, Apr 30, 2010 at 7:41 PM, Garrett Cooper <yan...@gm...> wrote: > On Fri, Apr 30, 2010 at 3:56 PM, Henry Yei <hy...@mv...> wrote: >> If a sudo file exists, the patch doesn't touch it. If it does not, a >> sudoers file will be created and then it will remove it afterwards. I >> wouldn't feel confortable having a script modify a sudoers file if it >> already exists. The sudoers content that it does put in place just >> enables the root user to use sudo, which would not give it any more >> access than it already has. This text is available in every sudoers >> file I've looked at. >> >> >> >> While I was testing the patch with the ltp-dev branch, I noticed that >> the -n option was put in front of every sudo call in commit >> 2aa40f7e10518977881b933cc93b8f50847cf3cf in order to suppress the >> interactive password check. >> >> However, neither sudo 1.6.8p12 from MontaVista Linux 6 or 1.6.9p17 >> from Ubuntu 9.04 supports the -n option. The patch also removes the >> -n option from sudo calls. This option was added in 3/2008. > > Ugh. The sudo in gentoo did this though and that's the reason why I > added it in some scripts -- otherwise the pieces of junk halt waiting > for user input and I had manually kill the scripts >:(... I guess a > test will need to be added to ensure that sudo _does_ have the -n > option beforehand, if it does, add the -n option. Otherwise all people > running older versions are kind of SoL. > > sudo is still a non-standard tool and a pain in the butt to deal with > in scripts as it changes from time to time. > > FWIW your patch only deals with the case when the script `exits > cleanly' as there aren't any traps installed for cleanup_test. > >> On Fri, Apr 30, 2010 at 1:15 PM, Garrett Cooper <yan...@gm...> wrote: >>> On Fri, Apr 30, 2010 at 12:27 PM, Henry Yei <hy...@mv...> wrote: >>>> Speaking of the utimesat test and prerequisites, our test systems >>>> don't have a sudoers file by default, so we have an internal patch to >>>> utimesat to create a default one if non exist and remove it after the >>>> test is done. Would that be of interest to LTP? >>>> >>>> On Tue, Apr 27, 2010 at 11:27 PM, Caspar Zhang <cas...@gm...> wrote: >>>>> On Wed, Apr 28, 2010 at 2:04 PM, Garrett Cooper <yan...@gm...> wrote: >>>>>> The first item is different from the other two. The first one ignores the >>>>>> signal (SIG_IGN), whereas the latter two cases reset the handler to the >>>>>> default one (SIG_DFL). I prefer the former format, because otherwise the >>>>>> signal handlers become reentrable on accident. >>>>> >>>>> I see. Thank you. >>>>> >>>>> The final patch ;-) >>> >>> If it: >>> >>> 1. Works in all cases, i.e. doesn't use version specific constructs >>> for sudo (which I haven't seen thus far, but just to be safe). >>> 2. Is properly reverted when the test is done (which includes the >>> following scenarios): >>> a. File already exists. Backup the old file, revert it when the >>> test is completed (regardless of whether or not the test passed or the >>> test failed properly [*]). >>> b. File doesn't exist. Nuke the file after the test is done. >>> >>> ... sure. >>> Thanks, >>> -Garrett >>> >>> [*] SIGKILL or SIGSTOP can't be avoided, so technically it's a best effort. > > Thanks, > -Garrett > |