Re: [LTP] [PATCH] fix lchown tests

[Garrett C. <yan...@gm...>]

On Fri, Nov 26, 2010 at 6:27 AM, Cyril Hrubis <ch...@su...> wrote:

...

> Well, the return 0; is there just to silent compiler as cleanup() calls
> tst_exit(); The best solution is IMHO add noreturn attribute to
> cleanup() and remove the return 0;

Eventually what will happen (if you've been watching git) is that the
calls will become:

cleanup();
tst_exit();

in main, and any and all references of tst_exit() elsewhere should be
replaced / removed. I've completed about 100 or so of the syscalls and
have about 800 to go. Whee...

>> +     if (close(fd) == -1)
>> +             tst_brkm(TBROK | TERRNO, cleanup, "close(2) %s", TESTFILE);
>>
>> gcooper> Wouldn't worry about the close.
>
> Checking it doesn't do any harm. ;)

Yeah. It's closing before running a test not after (and that's the
only potential exception for looking at the output of *close).
Sorry...

>>
>> -     /* fix permissions on the tmpdir */
>> -     if (chmod(".", 0711) != 0) {
>> -             tst_brkm(TBROK | TERRNO, cleanup, "chmod() failed");
>>
>> gcooper> The directory mode for tst_tmpdir is different:
>>
>> #define DIR_MODE        0777  /* mode of tmp dir that will be created */
>
> As far as I understand the tests, changing permissions on tmp directory
> is not needed as the test is testing permissions on symlinks created
> inside of it (so we only need that the content of the directory is
> accesible). I susspect that this is some workaround that wasn't removed
> when test was ported for LTP.

Are you sure? If you change credentials (and this test does) the
contents of the directory are inaccessible. I'd readd this and check
what the outcome is of the test.
Thanks,
-Garrett