Compile warning fix#237
Conversation
|
Thanks for this PR. The "Assisted-by" tags are interesting. These will be the first commits with these tags, probably :) The src/Makefile change conflicts with the recent PR #238. I will work on conflict resolution tomorrow. |
kawasaki
left a comment
There was a problem hiding this comment.
@yizhanglinux Thanks for this PR. I made two comments on the two patches. Could you check up the comments?
| if (v != ref) { | ||
| fprintf(stderr, "reftag interval:%d expected:%x got:%llx\n", | ||
| fprintf(stderr, "reftag interval:%d expected:%x got:%" PRIx64 "\n", | ||
| i, ref, v); |
There was a problem hiding this comment.
When I applied this patch on my x86 system, C compilter reported the warning below:
make -C src all
make[1]: Entering directory '/home/shin/Blktests/blktests/src'
cc -O2 -Wall -Wshadow -DHAVE_LINUX_BLKZONED_H -D_GNU_SOURCE -o metadata metadata.c -lpthread -luring
metadata.c: In function ‘check_metadata’:
metadata.c:249:57: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘__u64’ {aka ‘long long unsigned int’} [-Wformat=]
249 | fprintf(stderr, "reftag interval:%d expected:%x got:%" PRIx64 "\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
250 | i, ref, v);
| ~
| |
| __u64 {aka long long unsigned int}
In file included from metadata.c:12:
/usr/include/inttypes.h:121:41: note: format string is defined here
121 | # define PRIx64 __PRI64_PREFIX "x"
make[1]: Leaving directory '/home/shin/Blktests/blktests/src'
On this system, both "__u64 = long long unsigned int" and "uint64_t = long unsigned int" have 64 bits size. PRIx64 expects uint64_t = long unsigned int, but __u64 is equivalent to long long unsinged int. GCC handled these two as different types and reported the warning.
To supress this warning, I suggest to add a type cast to the variable v:
iff --git a/src/metadata.c b/src/metadata.c
index ee11b18..1f50c20 100644
--- a/src/metadata.c
+++ b/src/metadata.c
@@ -247,7 +247,7 @@ static int check_metadata(void *p, int intervals, int ref)
if (v != ref) {
fprintf(stderr, "reftag interval:%d expected:%x got:%" PRIx64 "\n",
- i, ref, v);
+ i, ref, (uint64_t)v);
return -1;
}
remaining -= sizeof(*tuple);
There was a problem hiding this comment.
@kawasaki
On the 64-bit system, uint64_t is defined as a long unsigned int[1]. And Gemini suggests fixing it with [2], and I tried it on the ppc64le system and it works.
# uname -r
7.0.0-0.rc6.260401g9147566d8016.51.fc45.ppc64le
# gcc --version
gcc (GCC) 16.0.1 20260321 (Red Hat 16.0.1-0)
metadata.c: In function ‘check_metadata’:
metadata.c:248:96: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 5 has type ‘long unsigned int’ [-Wformat=]
248 | fprintf(stderr, "reftag interval:%d expected:%x got:%llx\n",
| ~~~^
| |
| long long unsigned int
| %lx
249 | i, ref, (uint64_t)v);
| ~~~~~~~~~~~
| |
| long unsigned int
[2]
diff --git a/src/metadata.c b/src/metadata.c
index d935fd6..556f369 100644
--- a/src/metadata.c
+++ b/src/metadata.c
@@ -246,7 +246,7 @@ static int check_metadata(void *p, int intervals, int ref)
if (v != ref) {
fprintf(stderr, "reftag interval:%d expected:%x got:%llx\n",
- i, ref, v);
+ i, ref, (unsigned long long)v);
return -1;
}
remaining -= sizeof(*tuple);
|
|
||
| $(C_TARGETS): %: %.c | ||
| $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $^ | ||
|
|
There was a problem hiding this comment.
Thank you for the patch. This patch conflicts with dd55eba.
Based on this patch, I suggest the change below to resolve the conflict.
diff --git a/src/Makefile b/src/Makefile
index 10da40d..d8833bf 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -19,7 +19,6 @@ C_TARGETS := \
nbdsetsize \
openclose \
sg/dxfer-from-dev \
- sg/syzkaller1 \
zbdioctl
C_URING_TARGETS := metadata \
@@ -32,7 +31,10 @@ HAVE_UBLK_HEADER := $(call HAVE_C_HEADER,linux/ublk_cmd.h,1)
CXX_TARGETS := \
discontiguous-io
-TARGETS := $(C_TARGETS) $(CXX_TARGETS)
+SYZKALLER_TARGETS := \
+ sg/syzkaller1
+
+TARGETS := $(C_TARGETS) $(CXX_TARGETS) $(SYZKALLER_TARGETS)
ifeq ($(HAVE_UBLK_HEADER), 1)
C_URING_TARGETS += $(C_UBLK_TARGETS)
@@ -70,6 +72,9 @@ $(C_TARGETS): %: %.c
$(CXX_TARGETS): %: %.cpp
$(CXX) $(CPPFLAGS) $(CXXFLAGS) $(LDFLAGS) -o $@ $^
+$(SYZKALLER_TARGETS): %: %.c
+ $(CC) $(CFLAGS) -Wno-unused-but-set-variable $(LDFLAGS) -o $@ $^
+
$(C_URING_TARGETS): %: %.c
$(CC) $(CFLAGS) $(LDFLAGS) $(URING_FLAGS) -o $@ $^ $(URING_LIBS)
There was a problem hiding this comment.
Thanks, I've updated it.
On ppc64le, __u64 is typedef'd as 'unsigned long' rather than 'unsigned long long', so cast to unsigned long long fix the warning. Fixes: linux-blktests#233 Assisted-by: Gemini Signed-off-by: Yi Zhang <[email protected]>
…er1.c The volatile loop variable 'i' in sg/syzkaller1.c triggers a -Wunused-but-set-variable warning on newer GCC (e.g. GCC 16 on ppc64le). So add a specific Makefile rule for sg/syzkaller1 that appends -Wno-unused-but-set-variable. Fixes: linux-blktests#233 Suggested-by: Shin'ichiro Kawasaki <[email protected]> Signed-off-by: Yi Zhang <[email protected]>
84c4e9e to
d5e7400
Compare
|
I have merged this PR. Thanks! |
No description provided.