Re: [LTP] [PATCH 1/3] fcntl14: dup code clean up

[Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>]

Hi!

On 09/19/2014 10:46 AM, Xiong Zhou wrote:
>
>
> ----- Original Message -----
>> From: "Stanislav Kholmanskikh" <stanislav.kholmanskikh@oracle.com>
>> To: "Xiong Zhou" <xzhou@redhat.com>, ltp-list@lists.sourceforge.net
>> Sent: Thursday, September 11, 2014 10:04:32 PM
>> Subject: Re: [LTP] [PATCH 1/3] fcntl14: dup code clean up
>>
>> Hi!
>>
>> On 09/11/2014 02:30 PM, Xiong Zhou wrote:
>>> And block3 description message fix from "mandatory locking"
>>> to "negative whence".
>>>
>>> Signed-off-by: Xiong Zhou <xzhou@redhat.com>
>>> ---
>>>    testcases/kernel/syscalls/fcntl/fcntl14.c | 36
>>>    +++++++++++--------------------
>>>    1 file changed, 13 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/testcases/kernel/syscalls/fcntl/fcntl14.c
>>> b/testcases/kernel/syscalls/fcntl/fcntl14.c
>>> index 30a1ea9..9db88e8 100644
>>> --- a/testcases/kernel/syscalls/fcntl/fcntl14.c
>>> +++ b/testcases/kernel/syscalls/fcntl/fcntl14.c
>>> @@ -969,6 +969,15 @@ void catch1(int sig)
>>>    	got1++;
>>>    }
>>>
>>> +static void testcheck_end(int check_fail, char *msg)
>>> +{
>>> +	if (check_fail) {
>>> +		tst_resm(TFAIL, "%s FAILED", msg);
>>> +	} else {
>>> +		tst_resm(TPASS, "%s PASSED", msg);
>>> +	}
>>> +}
>>
>> There are many places left in fcntl14.c where you can put this
>> "deduplication". Why aren't you doing it? ;)
>
> I think I have done all of them, I cannot find other places which are applicable to this function.

Sorry, but by "many places" I meant block 2.

Last time I commented your [PATCH 2/3] with "And testcheck_end() to 
block 2 should be introduced in the previous patch."

Could you please add all testcheck_end() invocations in the first patch 
of the series?

Thanks.


>
> I will resend this series based on your last comments.
>
> Thanks.
>
>>
>>> +
>>>    int main(int ac, char **av)
>>>    {
>>>    	int lc;
>>> @@ -1009,20 +1018,11 @@ int main(int ac, char **av)
>>>    		 * mandatory locking
>>>    		 */
>>>    		(void)run_test(O_CREAT | O_RDWR | O_TRUNC, 0777, 0, 0, 36);
>>> -		if (fail) {
>>> -			tst_resm(TFAIL, "Block 1, test 1 FAILED");
>>> -		} else {
>>> -			tst_resm(TPASS, "Block 1, test 1 PASSED");
>>> -		}
>>> +		testcheck_end(fail, "Block 1, test 1");
>>>
>>>    		/* Now try with negative values for L_start and L_len */
>>>    		(void)run_test(O_CREAT | O_RDWR | O_TRUNC, 0777, 5, 36, 45);
>>> -
>>> -		if (fail) {
>>> -			tst_resm(TFAIL, "Block 1, test 2 FAILED");
>>> -		} else {
>>> -			tst_resm(TPASS, "Block 1, test 2 PASSED");
>>> -		}
>>> +		testcheck_end(fail, "Block 1, test 2");
>>>
>>>    		tst_resm(TINFO, "Exit block 1");
>>>
>>> @@ -1094,13 +1094,7 @@ int main(int ac, char **av)
>>>    		close(fd);
>>>    		unlink(tmpname);
>>>
>>> -		if (fail) {
>>> -			tst_resm(TINFO, "Test with mandatory "
>>> -				 "locking FAILED");
>>> -		} else {
>>> -			tst_resm(TINFO, "Test with mandatory "
>>> -				 "locking PASSED");
>>> -		}
>>> +		testcheck_end(fail, "Test with negative whence locking");
>>>    		tst_resm(TINFO, "Exit block 3");
>>>
>>>    /* //block4: */
>>> @@ -1223,11 +1217,7 @@ int main(int ac, char **av)
>>>    		close(fd);
>>>    		unlink(tmpname);
>>>
>>> -		if (fail) {
>>> -			tst_resm(TINFO, "Test of locks on file FAILED");
>>> -		} else {
>>> -			tst_resm(TINFO, "Test of locks on file PASSED");
>>> -		}
>>> +		testcheck_end(fail, "Test of locks on file");
>>>    		tst_resm(TINFO, "Exit block 4");
>>>    	}
>>>    	cleanup();
>>>
>>

------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list