Re: [LTP] [PATCH] mmap/munmap /dev/zero

[Mike F. <va...@ge...>]

On Tuesday, October 12, 2010 10:07:16 CAI Qian wrote:
> +/* main setup function of test */
> +void setup();
> +/* cleanup function for the test */
> +void cleanup();
> +int mmapzero();

functins that take no args should be "(void)" and not "()"

> +int main(int ac, char **av)

use consistent style.  this should be "int argc" and "char *argv[]".

> +	if (msg != (char *)NULL) {

useless cast

> +		if(TEST_RETURN != 0)

need a space before the "("

> +	 x = mmap("/dev/zero", SIZE+SIZE-4096, PROT_READ|PROT_WRITE,
> +		 MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);

uhh, have you even checked this test ?  this mmap() makes absolutely no sense 
on so many levels.

> +	 if (x == MAP_FAILED) {
> +		 perror("error");

make this perror() slightly less useless by replacing "error" with "mmap".  
although, i wonder why you cant use any of the ltp helper functions for error 
reporting ... tests rarely (if ever) should be writing to stdout/stderr 
themselves.

> +	 if (!fork()) {
> +		 munmap(x + SIZE+4096, SIZE-4096*2);
> +		 _exit(0);
> +	 }
> +	 if (!fork()) {
> +		 if (!fork()) {
> +			 munmap(x + SIZE+4096, SIZE-4096*2);
> +			 _exit(0);
> +		 }
> +		 munmap(x + SIZE+4096, SIZE-4096*2);
> +		 _exit(0);
> +	 }
> +	 munmap(x, SIZE+SIZE-4096);

fork() returns -1 on error which means this code misbehaves.

why are you using _exit() ?

munmap() returns a value you shouldbe checking.  blindly calling funcs doesnt 
accomplish much.

> + void cleanup()
> + {
> +	 /*
> +	  * remove the tmp directory and exit
> +	  */
> +	 TEST_CLEANUP;
> +	 tst_rmdir();
> +	 tst_exit();
> +
> + }

useless newline before the closing brace
-mike