Re: [LTP] [PATCH v3] fw_load: new test of device firmware loading

[chrubis@suse.cz]

Hi!
> I thought it is not necessary because WITH_MODULES won't be set if make
> target is clean.

Ah, the config.mk is include omited on 'make clean'. However I think
that depending on this is too confusing.

> >> +	if (access(module_name, F_OK) == -1) {
> >> +		tst_brkm(TCONF, NULL,
> >> +			"Test requires kernel module '%s'", module_name);
> >> +	}
> > Here you expect that the module is in current directory, which is not
> > robust enough.
> >
> > What about creating module_exists() module_load() and module_unload()
> > library functions?
> Yeah, it is possible. Could I use tst_resource files for that? Or perhaps,
> would it be better to create tst_modules files?

Start a separate source file for that, tst_module(s) is OK.

> > The path to the module should be determined from LTPROOT env variable,
> > see lib/tst_resource.c and lib/tst_resource.h that handles similar
> > situation for files needed by the tests.
> >
> >
> >> +
> >> +static void cleanup(void)
> >> +{
> >> +	if (skip_cleanup)
> >> +		return;
> >> +	int i;
> >> +	for (i = fw_num - 1; i>= 0; --i) {
> > Is there any reason for the loop going backward?
> >
> It is the only way to successfully remove created directories.
> I need to start from the last created file or/and directory and move
> upward.

I see, some of the dirs are subdirs of previously created dirs. What
about adding a short comment explaining that?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list