Re: [LTP] [PATCH 1/3] syscalls: refactor fork09 using new API

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

Generally LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

> +	tst_res(TINFO, "Opening files from parent");
> +
> +	for (nfiles = 0; nfiles < file_open_max; nfiles++) {
> +		memset(name, 0, PATH_MAX);
> +		snprintf(name, PATH_MAX, "%s%lu", FILE_PREFIX, nfiles);

> -		/* clean up things in case we are looping */
> -		for (nf = first; nf < nfiles; nf++) {
> -			fclose(fildeses[nf]);
> -			sprintf(filname, "file%d.%d", nf, mypid);
> -			unlink(filname);
> +		f = fopen(name, "a");
> +		if (!f) {
nit: Although EMFILE meaning is quite obvious I found previous comment about
OPEN_MAX useful.

> +			if (errno == EMFILE)
> +				break;
> +
> +			tst_brk(TBROK | TERRNO, "fopen() error");
I suppose we are ok to not close these open files even if it fails close to
OPEN_MAX...

>  		}
> +
> +		open_files[nfiles] = f;
> +	}
> +
> +	totfiles = nfiles;
> +
> +	if (!totfiles)
> +		tst_brk(TBROK, "Parent couldn't open any file");
> +
> +	tst_res(TINFO, "Closing %lu files from child", totfiles);
> +
> +	if (!SAFE_FORK()) {
> +		for (nfiles = nfiles - 1; nfiles >= 0; nfiles--)
> +			SAFE_FCLOSE(open_files[nfiles]);
> +
> +		exit(0);
>  	}

> +	tst_reap_children();
> +
> +	tst_res(TPASS, "Child closed all parent's files");
> +

... because this is not in a cleanup.
> +	for (nfiles = 0; nfiles < totfiles; nfiles++) {
> +		memset(name, 0, PATH_MAX);
> +		snprintf(name, PATH_MAX, "%s%lu", FILE_PREFIX, nfiles);
> +
> +		SAFE_FCLOSE(open_files[nfiles]);
> +		SAFE_UNLINK(name);
> +	}

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp