On 06/18/2013 04:28 PM, [email protected] wrote: > > Also you initialize PROCEED variable but then go ahead and use lowercase > proceed instead. You should stick to uppercase. OK, I'll fix it. > And isn't the proceed value set if make target is clean replaced with > whatever we get later in the ifdefs? Did you mean proceed ?= $(shell > ...) there? > I thought it is not necessary because WITH_MODULES won't be set if make target is clean. I think, I will need to remove PROCEED initialization, if I set query-assignment. > >> + >> + 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? > 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. >> + if (fw[i].remove_file&& remove(fw[i].file) == -1) >> + tst_resm(TWARN, "Can't remove: %s", fw[i].file); >> + >> + if (fw[i].remove_dir&& remove(fw[i].dir) == -1) >> + tst_resm(TWARN, "Can't remove %s", fw[i].dir); >> + } >> + >> + if (module_registered) { >> + char cmd[MAX_CMD_LEN]; >> + snprintf(cmd, MAX_CMD_LEN, "rmmod %s", module_name); >> + if (system(cmd) != 0) >> + tst_brkm(TBROK, NULL, "Can't remove %s", module_name); >> + } >> + >> + TEST_CLEANUP; >> +} >> + >> +static void create_firmware(const char *path) >> +{ >> + int i; >> + for (i = 0; i< ARRAY_SIZE(fw_paths); ++i) { >> + struct fw_file_info *f =&fw[fw_num]; >> + snprintf(f->dir, PATH_MAX, "%s/%s", path, fw_paths[i]); >> + if (access(f->dir, X_OK) == -1) { >> + /* create dir */ >> + SAFE_MKDIR(cleanup, f->dir, 0755); >> + f->remove_dir = 1; >> + } >> + >> + /* create test firmware file */ >> + snprintf(f->file, PATH_MAX, "%s/n%d_%s", >> + f->dir, fw_num, fw_name); >> + >> + FILE *fd = fopen(f->file, "w"); > Naming FILE * variable fd is a little confusing as it suggests file > descriptor. I would settle on simple f instead. >> + if (fd == NULL) >> + tst_brkm(TBROK, cleanup, "Failed to create firmware"); >> + int k; >> + for (k = 0; k< fw_size; ++k) >> + fputc(fw_num, fd); >> + fclose(fd); > It's important to check the return value from fclose() here, as the > output is buffered and the data flushed at the fclose() call. I've > looked into the safe_macros.h and it looks like we do not have > SAFE_FCLOSE() yet. I will add the FILE * calls there so that you can use > here. > OK
------------------------------------------------------------------------------ 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
