| Message ID | 20230929112039.7488-2-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | selftests/resctrl: Fixes to failing tests | expand |
Hi Ilpo, On 9/29/2023 4:20 AM, Ilpo Järvinen wrote: > signal_handler_unregister() calls sigaction() with uninitializing > sa_flags in the struct sigaction. > > Make sure sa_flags is always initialized in signal_handler_unregister() > by initializing the struct sigaction when declaring it. > > Fixes: 73c55fa5ab55 (selftests/resctrl: Commonize the signal handler register/unregister for all tests) Please place the title line in quotes (checkpatch warning). > Suggested-by: Reinette Chatre <reinette.chatre@intel.com> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > Cc: <stable@vger.kernel.org> > --- > tools/testing/selftests/resctrl/resctrl_val.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c > index 51963a6f2186..1e8b90077218 100644 > --- a/tools/testing/selftests/resctrl/resctrl_val.c > +++ b/tools/testing/selftests/resctrl/resctrl_val.c > @@ -504,7 +504,7 @@ int signal_handler_register(void) > */ > void signal_handler_unregister(void) > { > - struct sigaction sigact; > + struct sigaction sigact = {}; > > sigact.sa_handler = SIG_DFL; > sigemptyset(&sigact.sa_mask); Could you please add this initialization to signal_handler_register() also? I understand that the particular issue of sa_flags is not relevant to that function but there are other uninitialized fields. I think initializing the struct makes the code more robust without needing to reason/understand glibc behavior. Reinette
On Fri, 29 Sep 2023, Reinette Chatre wrote: > Hi Ilpo, > > On 9/29/2023 4:20 AM, Ilpo Järvinen wrote: > > signal_handler_unregister() calls sigaction() with uninitializing > > sa_flags in the struct sigaction. > > > > Make sure sa_flags is always initialized in signal_handler_unregister() > > by initializing the struct sigaction when declaring it. > > > > Fixes: 73c55fa5ab55 (selftests/resctrl: Commonize the signal handler register/unregister for all tests) > > Please place the title line in quotes (checkpatch warning). Thanks. I hadn't realize the quotes vanished without an error when I removed the escape from " char in gitconfig alias (I have just migrated to use alias instead of a custom tool). > > Suggested-by: Reinette Chatre <reinette.chatre@intel.com> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > Cc: <stable@vger.kernel.org> > > --- > > tools/testing/selftests/resctrl/resctrl_val.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c > > index 51963a6f2186..1e8b90077218 100644 > > --- a/tools/testing/selftests/resctrl/resctrl_val.c > > +++ b/tools/testing/selftests/resctrl/resctrl_val.c > > @@ -504,7 +504,7 @@ int signal_handler_register(void) > > */ > > void signal_handler_unregister(void) > > { > > - struct sigaction sigact; > > + struct sigaction sigact = {}; > > > > sigact.sa_handler = SIG_DFL; > > sigemptyset(&sigact.sa_mask); > > Could you please add this initialization to signal_handler_register() > also? I understand that the particular issue of sa_flags is not > relevant to that function but there are other uninitialized fields. > I think initializing the struct makes the code more robust without > needing to reason/understand glibc behavior. Okay, I'll do that and add a note into the changelog that then correctness is obvious from the code itself.
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 51963a6f2186..1e8b90077218 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -504,7 +504,7 @@ int signal_handler_register(void) */ void signal_handler_unregister(void) { - struct sigaction sigact; + struct sigaction sigact = {}; sigact.sa_handler = SIG_DFL; sigemptyset(&sigact.sa_mask);
signal_handler_unregister() calls sigaction() with uninitializing sa_flags in the struct sigaction. Make sure sa_flags is always initialized in signal_handler_unregister() by initializing the struct sigaction when declaring it. Fixes: 73c55fa5ab55 (selftests/resctrl: Commonize the signal handler register/unregister for all tests) Suggested-by: Reinette Chatre <reinette.chatre@intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Cc: <stable@vger.kernel.org> --- tools/testing/selftests/resctrl/resctrl_val.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)