lkml.org 
[lkml]   [2021]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v4 01/17] selftests/resctrl: Fix compilation issues for global variables
From
Date
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

\
 
 \ /
  Last update: 2021-01-26 11:33    [W:0.209 / U:0.828 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site