Messages in this thread | | | Date | Thu, 21 Apr 2022 17:21:27 +0100 | Subject | Re: [PATCH 2/3] perf test: Shell - only run .sh shell files to skip other files | From | Carsten Haitzler <> |
| |
On 4/10/22 03:28, Leo Yan wrote: > On Wed, Mar 09, 2022 at 12:28:58PM +0000, carsten.haitzler@foss.arm.com wrote: >> From: Carsten Haitzler <carsten.haitzler@arm.com> >> >> You edit your scripts in the tests and end up with your usual shell >> backup files with ~ or .bak or something else at the end, but then your >> next perf test run wants to run the backups too. You might also have perf >> .data files in the directory or something else undesireable as well. You end >> up chasing which test is the one you edited and the backup and have to keep >> removing all the backup files, so automatically skip any files that are >> not plain *.sh scripts to limit the time wasted in chasing ghosts. >> >> Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com> >> >> --- >> tools/perf/tests/builtin-test.c | 17 +++++++++++++++-- >> 1 file changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c >> index 3c34cb766724..3a02ba7a7a89 100644 >> --- a/tools/perf/tests/builtin-test.c >> +++ b/tools/perf/tests/builtin-test.c >> @@ -296,9 +296,22 @@ static const char *shell_test__description(char *description, size_t size, >> >> #define for_each_shell_test(entlist, nr, base, ent) \ >> for (int __i = 0; __i < nr && (ent = entlist[__i]); __i++) \ >> - if (!is_directory(base, ent) && \ >> + if (ent->d_name[0] != '.' && \ >> + !is_directory(base, ent) && \ >> is_executable_file(base, ent) && \ >> - ent->d_name[0] != '.') >> + is_shell_script(ent->d_name)) > > Just nitpick: since multiple conditions are added, seems to me it's good > to use a single function is_executable_shell_script() to make decision > if a file is an executable shell script.
I'd certainly make a function if this was being re-used, but as the "coding pattern" was to do all the tests already inside the if() in only one place, I kept with the style there and didn't change the code that didn't need changing. I can rewrite this code and basically make a function that is just an if ...:
bool is_exe_shell_script(const char *base, struct dirent *ent) { return ent->d_name[0] != '.' && !is_directory(base, ent) && is_executable_file(base, ent) && is_shell_script(ent->d_name); }
And macro becomes:
#define for_each_shell_test(entlist, nr, base, ent) \ for (int __i = 0; __i < nr && (ent = entlist[__i]); __i++) \ if (is_shell(base, ent))
But one catch... it really should be is_non_hidden_exe_shell_script() as it's checking that it's not a hidden file AND is a shell script. Or do I keep the hidden file test outside of the function in the if? If we're nit picking then I need to know exactly what you want here as your suggested name is actually incorrect.
> And the condition checking 'ent->d_name[0] != '.'' would be redundant > after we have checked the file suffix '.sh'.
This isn't actually redundant. You can have .something.sh :) If the idea is we skip anything with a . at the start first always... then the if (to me) is obvious.
> Thanks, > Leo > >> + >> +static bool is_shell_script(const char *file) >> +{ >> + const char *ext; >> + >> + ext = strrchr(file, '.'); >> + if (!ext) >> + return false; >> + if (!strcmp(ext, ".sh")) >> + return true; >> + return false; >> +} >> >> static const char *shell_tests__dir(char *path, size_t size) >> { >> -- >> 2.32.0 >>
| |