On 02/06/2012 09:44 AM, Caspar Zhang wrote:
> On 02/03/2012 09:31 AM, Wanlong Gao wrote:
>> cleanup the coding style
>>
>> Signed-off-by: Wanlong Gao <[email protected]>
>> ---
>> testcases/kernel/mem/hugetlb/hugemmap/hugemmap03.c | 132
>> +++++++++-----------
>> 1 files changed, 60 insertions(+), 72 deletions(-)
>>
>> diff --git a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap03.c
>> b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap03.c
>> index cac94b0..93638c1 100644
>> --- a/testcases/kernel/mem/hugetlb/hugemmap/hugemmap03.c
>> +++ b/testcases/kernel/mem/hugetlb/hugemmap/hugemmap03.c
>> @@ -23,17 +23,8 @@
>> * Test Description: Test that a normal page cannot be mapped into a high
>> * memory region.
>> *
>> - * Usage: <for command-line>
>> - * hugemmap03 [-c n] [-f] [-i n] [-I x] [-P x] [-t]
>> - * where, -c n : Run n copies concurrently.
>> - * -f : Turn off functionality Testing.
>> - * -i n : Execute test n times.
>> - * -I x : Execute test for x seconds.
>> - * -P x : Pause for x seconds between iterations.
>> - * -t : Turn on syscall timing.
>> - *
>> * HISTORY
>> - * 04/2004 Written by Robbie Williamson
>> + * 04/2004 Written by Robbie Williamson
>> *
>> * RESTRICTIONS:
>> * Must be compiled in 64-bit mode.
>> @@ -57,76 +48,69 @@
>>
>> #define HIGH_ADDR (void *)(0x1000000000000)
>>
>> -char* TEMPFILE="mmapfile";
>> -
>> -char *TCID="hugemmap03"; /* Test program identifier. */
>> -int TST_TOTAL=1; /* Total number of test cases. */
>> -unsigned long *addr; /* addr of memory mapped region */
>> -int fildes; /* file descriptor for tempfile */
>> -char *Hopt; /* location of hugetlbfs */
>> +static char *TEMPFILE = "mmapfile";
>>
>> -void setup(); /* Main setup function of test */
>> -void cleanup(); /* cleanup function for the test */
>> +char *TCID = "hugemmap03";
>> +int TST_TOTAL = 1;
>> +static unsigned long *addr;
>> +static int fildes;
>> +static char *Hopt;
>>
>> -void help()
>> -{
>> - printf(" -H /.. Location of hugetlbfs, i.e. -H /var/hugetlbfs \n");
>> -}
>> +static void setup(void);
>> +static void cleanup(void);
>> +static void help(void);
>>
>> -int
>> -main(int ac, char **av)
>> +int main(int ac, char **av)
>> {
>> - int lc; /* loop counter */
>> - char *msg; /* message returned from parse_opts */
>> - int Hflag=0; /* binary flag: opt or not */
>> + int lc;
>> + char *msg;
>> + int Hflag = 0;
>> int page_sz;
>>
>> -#if __WORDSIZE==32 /* 32-bit compiled */
>> +#if __WORDSIZE == 32 /* 32-bit compiled */
>> tst_brkm(TCONF, NULL, "This test is only for 64bit");
>> #endif
>>
>> - option_t options[] = {
>> - { "H:", &Hflag, &Hopt }, /* Required for location of
>> hugetlbfs */
>> - { NULL, NULL, NULL } /* NULL required to end
>> array */
>> - };
>> + option_t options[] = {
>> + { "H:", &Hflag, &Hopt },
>> + { NULL, NULL, NULL }
>> + };
>>
>> - /* Parse standard options given to run the test. */
>> msg = parse_opts(ac, av, options, &help);
>> - if (msg != (char *) NULL) {
>> - tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s, use -help",
>> msg);
>> - tst_exit();
>> - }
>> + if (msg != NULL)
>> + tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s, use -help",
>> + msg);
>>
>> - if (Hflag == 0) {
>> - tst_brkm(TBROK, NULL, "-H option is REQUIRED for this test, use
>> -h for options help");
>> - tst_exit();
>> - }
>> + if (!Hflag)
>> + tst_brkm(TBROK, NULL, "-H option is REQUIRED for this test, "
>> + "use -h for options help");
>>
>> page_sz = getpagesize();
>>
>> setup();
>>
>> for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -
>> - /* Creat a temporary file used for huge mapping */
>> - if ((fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0666)) < 0) {
>> - tst_brkm(TFAIL, cleanup,
>> - "open() on %s Failed, errno=%d : %s",
>> - TEMPFILE, errno, strerror(errno));
>> - }
>> -
>> - Tst_count=0;
>> -
>> - /* Attempt to mmap using normal pages and a high memory address
>> */
>> - errno = 0;
>> + /* Creat a temporary file used for huge mapping */
>> + fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0666);
>> + if (fildes < 0)
>> + tst_brkm(TBROK|TERRNO, cleanup,
>> + "opening %s failed", TEMPFILE);
>> +
>> + Tst_count = 0;
>> +
>> + /*
>> + * Attempt to mmap using normal pages and
>> + * a high memory address
>> + */
>> addr = mmap(HIGH_ADDR, page_sz, PROT_READ,
>> MAP_SHARED | MAP_FIXED, fildes, 0);
>> if (addr != MAP_FAILED) {
>> - tst_resm(TFAIL, "Normal mmap() into high region
>> unexpectedly succeeded on %s, errno=%d : %s",
>> - TEMPFILE, errno, strerror(errno));
>> + tst_resm(TFAIL|TERRNO, "Normal mmap() into high region"
>> + " unexpectedly succeeded on %s, TEMPFILE");
>> continue;
>> } else {
>> - tst_resm(TPASS, "Normal mmap() into high region failed
>> correctly");
>> + tst_resm(TPASS, "Normal mmap() into high region"
>> + " failed correctly");
>> break;
>> }
>>
>> @@ -141,21 +125,21 @@ main(int ac, char **av)
>> /*
>> * setup() - performs all ONE TIME setup for this test.
>> *
>> - * Get system page size, allocate and initialize the string dummy.
>> - * Initialize addr such that it is more than one page below the break
>> - * address of the process, and initialize one page region from addr
>> - * with char 'A'.
>> - * Creat a temporary directory and a file under it.
>> - * Write some known data into file and get the size of the file.
>> + * Get system page size, allocate and initialize the string dummy.
>> + * Initialize addr such that it is more than one page below the break
>> + * address of the process, and initialize one page region from addr
>> + * with char 'A'.
>> + *
>> + * Creat a temporary directory and a file under it.
>> + * Write some known data into file and get the size of the file.
>> */
>
> I personally think the comments for setup() and cleanup() are kind of
> useless, so I'd suggest remove all of them from 4 patches.
yeah, agree.
Thanks
-Wanlong Gao
>
>> -void
>> -setup()
>> +void setup(void)
>
> Here seems you missed a `static`
>
>> {
>> char mypid[40];
>>
>> - sprintf(mypid,"/%d",getpid());
>> - TEMPFILE=strcat(mypid,TEMPFILE);
>> - TEMPFILE=strcat(Hopt,TEMPFILE);
>> + sprintf(mypid, "/%d", getpid());
>> + TEMPFILE = strcat(mypid, TEMPFILE);
>> + TEMPFILE = strcat(Hopt, TEMPFILE);
>>
>> tst_sig(FORK, DEF_HANDLER, cleanup);
>>
>> @@ -165,11 +149,10 @@ setup()
>>
>> /*
>> * cleanup() - performs all ONE TIME cleanup for this test at
>> - * completion or premature exit.
>> - * Remove the temporary directory created.
>> + * completion or premature exit.
>> + * Remove the temporary directory created.
>> */
>> -void
>> -cleanup()
>> +void cleanup(void)
>
> ditto
>
>> {
>> /*
>> * print timing stats if that option was specified.
>> @@ -179,3 +162,8 @@ cleanup()
>> unlink(TEMPFILE);
>>
>> }
>> +
>> +void help(void)
>
> ditto
>
>> +{
>> + printf(" -H /.. Location of hugetlbfs, i.e. -H /var/hugetlbfs\n");
>> +}
>
>
------------------------------------------------------------------------------
Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-dev2
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
