Messages in this thread Patch in this message | | | Subject | Re: [PATCH v4 01/17] selftests/resctrl: Fix compilation issues for global variables | From | Shuah Khan <> | Date | Mon, 25 Jan 2021 18:22:25 -0700 |
| |
On 11/30/20 1:19 PM, Fenghua Yu wrote: > Reinette reported following compilation issue on Fedora 32, gcc version > 10.1.1 > > /usr/bin/ld: cqm_test.o:<src_dir>/cqm_test.c:22: multiple definition of > `cache_size'; cat_test.o:<src_dir>/cat_test.c:23: first defined here > > The same issue is reported for long_mask, cbm_mask, count_of_bits etc > variables as well. Compiler isn't happy because these variables are > defined globally in two .c files namely cqm_test.c and cat_test.c and > the compiler during compilation finds that the variable is already > defined (multiple definition error). > > Taking a closer look at the usage of these variables reveals that these > variables are used only locally to functions such as cqm_resctrl_val() > (defined in cqm_test.c) and cat_perf_miss_val() (defined in cat_test.c). > These variables are not shared between those functions. So, there is no > need for these variables to be global. Hence, fix this issue by making > them local variables to the functions where they are used. >
Easy fix to this problem would be making these variables static to these files. I am not seeing any real advantage to changing these variable to local variables.
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 5da43767b973..360456b8a1b6 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -17,10 +17,10 @@ #define MAX_DIFF_PERCENT 4 #define MAX_DIFF 1000000
-int count_of_bits; -char cbm_mask[256]; -unsigned long long_mask; -unsigned long cache_size; +static int count_of_bits; +static cbm_mask[256]; +static unsigned long long_mask; +static unsigned long cache_size;
Same changes made to tools/testing/selftests/resctrl/cqm_test.c
> To fix issues for other global variables (e.g: bm_pid, ppid, llc_occup_path > and is_amd) that are used across .c files, declare them as extern. >
This change is fine. Make this a separate patch. This way, we can take it as a fixes to stables.
With the above the test builds fine. There is a bigger problem that needs fixing: (thix might have been fixed in later patches perhaps):
cqm_test.c: In function ‘check_results’: cqm_test.c:89:9: warning: ‘fgets’ writing 1024 bytes into a region of size 512 overflows the destination [-Wstringop-overflow=] 89 | while (fgets(temp, 1024, fp)) { | ^~~~~~~~~~~~~~~~~~~~~ In file included from resctrl.h:5, from cqm_test.c:11: /usr/include/stdio.h:568:14: note: in a call to function ‘fgets’ declared with attribute ‘write_only (1, 2)’ 568 | extern char *fgets (char *__restrict __s, int __n, FILE *__restrict __stream) | ^~~~~
thanks, -- Shuah
| |