Hi!
> create mode 100644 testcases/kernel/firmware/Makefile
> create mode 100644 testcases/kernel/firmware/fw_load_kernel/.gitignore
> create mode 100644 testcases/kernel/firmware/fw_load_kernel/Makefile
> create mode 100644 testcases/kernel/firmware/fw_load_kernel/README
> create mode 100644 testcases/kernel/firmware/fw_load_kernel/fw_load.c
> create mode 100644 testcases/kernel/firmware/fw_load_user/.gitignore
> create mode 100644 testcases/kernel/firmware/fw_load_user/Makefile
> create mode 100644 testcases/kernel/firmware/fw_load_user/README
> create mode 100644 testcases/kernel/firmware/fw_load_user/fw_load.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index eba0200..88471fa 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -314,6 +314,8 @@ ftruncate04_64 ftruncate04.sh 64
> #futimesat test cases
> futimesat01 futimesat01
>
> +fw_load fw_load
> +
> getcontext01 getcontext01
We created kernel_misc runtest file just lately and I think that this
test is better suited to be added to that runtest file rather than to
syscalls, do you agree?
> --- /dev/null
> +++ b/testcases/kernel/firmware/fw_load_kernel/.gitignore
> @@ -0,0 +1 @@
> +/fw_load.ko
Git status shows a bunch of other files generated by the build process,
maybe we should add these as well:
# Untracked files:
# (use "git add <file>..." to include in what will be committed)
#
# testcases/kernel/firmware/fw_load_kernel/.built-in.o.cmd
# testcases/kernel/firmware/fw_load_kernel/.fw_load.ko.cmd
# testcases/kernel/firmware/fw_load_kernel/.fw_load.mod.o.cmd
# testcases/kernel/firmware/fw_load_kernel/.fw_load.o.cmd
# testcases/kernel/firmware/fw_load_kernel/.tmp_versions/
# testcases/kernel/firmware/fw_load_kernel/Module.symvers
# testcases/kernel/firmware/fw_load_kernel/fw_load.mod.c
# testcases/kernel/firmware/fw_load_kernel/modules.order
At least *.o.cmd seems like a good pattern to ignore.
And perhaps it would be better to add the ltp_ prefix to the kernel
module name so that it's clear where it came from.
> +void setup(int argc, char *argv[])
> +{
> + char *msg;
> + msg = parse_opts(argc, argv, options, help);
> + if (msg != NULL)
> + tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
> +
> + if (nflag) {
> + if (sscanf(narg, "%i", &fw_size) != 1)
> + tst_brkm(TBROK, NULL, "-n option arg is not a number");
> + if (fw_size < 0)
> + tst_brkm(TBROK, NULL, "-n option arg is less than 0");
> + }
> +
> + tst_require_root(NULL);
> +
> + if (tst_kvercmp(3, 7, 0) < 0) {
> + tst_brkm(TCONF, NULL,
> + "Test must be run with kernel 3.7 or newer");
> + }
> +
> + char *mod_params[2] = {};
> + asprintf(&mod_params[0], "fw_size=%d", fw_size);
I would use array on the stack here as you know maximal size of the
string beforhand but this is very minor.
> + tst_module_load(NULL, module_name, mod_params);
> + free(mod_params[0]);
> +
> + tst_sig(FORK, DEF_HANDLER, cleanup);
> +
> + /* get current Linux version and make firmware paths */
> + struct utsname uts_name;
> + uname(&uts_name);
> + fw_paths[0] = strdup("firmware");
> + fw_paths[1] = strdup("firmware/updates");
As you are not freeing these strings the strdup() is useless here.
So either free these strings on exit or don't duplicated them here.
> + asprintf(&fw_paths[2], "firmware/%s", uts_name.release);
> + asprintf(&fw_paths[3], "firmware/updates/%s", uts_name.release);
> +
> + /* copy firmware to hard coded firmware search paths */
> + create_firmware("/lib");
> +
> + /* make non-existent firmware file */
> + asprintf(&fw[fw_num].file, "/n%d_%s", fw_num, fw_name);
> + fw[fw_num].fake = 1;
> + ++fw_num;
> +}
> +
> +static void test_run(void)
> +{
> + /* initiate firmware requests */
> + SAFE_FILE_PRINTF(cleanup, dev_fwnum, "%d", fw_num);
> +
> + /* get module results by reading result bit mask */
> + int result = 0;
> + SAFE_FILE_SCANF(cleanup, dev_result, "%d", &result);
> +
> + int i, fail, offset;
> + for (i = 0; i < fw_num; ++i) {
> + fail = (result & (1 << i)) == 0 && !fw[i].fake;
> + offset = (fw[i].dir) ? strlen(fw[i].dir) : 0;
> + tst_resm((fail) ? TFAIL : TPASS,
> + "Expect: %s load firmware '...%s'",
> + (fw[i].fake) ? "can't" : "can",
> + fw[i].file + offset);
> + }
> +}
> +
> +static void create_firmware(const char *path)
> +{
> + int i;
> + for (i = 0; i < ARRAY_SIZE(fw_paths); ++i) {
> + struct fw_file_info *fi = &fw[fw_num];
> + asprintf(&fi->dir, "%s/%s", path, fw_paths[i]);
> + if (access(fi->dir, X_OK) == -1) {
> + /* create dir */
> + SAFE_MKDIR(cleanup, fi->dir, 0755);
> + fi->remove_dir = 1;
> + }
> +
> + /* create test firmware file */
> + asprintf(&fi->file, "%s/n%d_%s", fi->dir, fw_num, fw_name);
> +
> + FILE *f = SAFE_FOPEN(cleanup, fi->file, "w");
> + if (f == NULL)
> + tst_brkm(TBROK, cleanup, "Failed to create firmware");
You will never get to the if (f == NULL) branch, safe operations will abort the
test and call cleanup callback in case something went wrong, that is the
whole point of the SAFE_ interface.
> + int k;
> + for (k = 0; k < fw_size; ++k)
> + fputc(fw_num, f);
> + SAFE_FCLOSE(cleanup, f);
> +
> + fi->remove_file = 1;
Just to be extra sure it gets removed, I think that you should turn this flag
on once the file was succesfully opened.
> + ++fw_num;
> + }
> +}
Apart from the small things the test looks good.
--
Cyril Hrubis
[email protected]
------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:
Build for Windows Store.
http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
