Re: [PATCH blktests v3 1/2] src: add mini ublk source code

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Feb 27, 2023 / 10:57, Ziyang Zhang wrote:
> On 2023/2/24 19:41, Shinichiro Kawasaki wrote:
> > On Feb 24, 2023 / 16:28, Ming Lei wrote:
> >> On Fri, Feb 24, 2023 at 03:52:28PM +0800, Ziyang Zhang wrote:
> >>> On 2023/2/20 11:46, Ming Lei wrote:
> >>>
> >>> [...]
> >>>
> >>>>
> >>>> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> >>>> ---
> >>>>  src/.gitignore |    1 +
> >>>>  src/Makefile   |   18 +
> >>>>  src/miniublk.c | 1376 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  3 files changed, 1395 insertions(+)
> >>>>  create mode 100644 src/miniublk.c
> >>>>
> >>>> diff --git a/src/.gitignore b/src/.gitignore
> >>>> index 355bed3..df7aff5 100644
> >>>> --- a/src/.gitignore
> >>>> +++ b/src/.gitignore
> >>>> @@ -8,3 +8,4 @@
> >>>>  /sg/dxfer-from-dev
> >>>>  /sg/syzkaller1
> >>>>  /zbdioctl
> >>>> +/miniublk
> >>>> diff --git a/src/Makefile b/src/Makefile
> >>>> index 3b587f6..81c6541 100644
> >>>> --- a/src/Makefile
> >>>> +++ b/src/Makefile
> >>>> @@ -2,6 +2,10 @@ HAVE_C_HEADER = $(shell if echo "\#include <$(1)>" |		\
> >>>>  		$(CC) -E - > /dev/null 2>&1; then echo "$(2)";	\
> >>>>  		else echo "$(3)"; fi)
> >>>>  
> >>>> +HAVE_C_MACRO = $(shell if echo "#include <$(1)>" |	\
> >>> Hi Ming,
> >>>
> >>> It should be "\#include", not "#include". You miss a "\".
> >>
> >> "\#include" won't work for checking the macro of IORING_OP_URING_CMD.
> >>
> >> [root@ktest-36 linux]# echo "\#include <liburing.h>" | gcc -E -
> >> # 0 "<stdin>"
> >> # 0 "<built-in>"
> >> # 0 "<command-line>"
> >> # 1 "/usr/include/stdc-predef.h" 1 3 4
> >> # 0 "<command-line>" 2
> >> # 1 "<stdin>"
> >> \#include <liburing.h>
> > 
> > I also tried and observed the same symptom. HAVE_C_MACRO works well without the
> > backslash. Adding the backslash, it fails.
> > 
> > I think Ziyang made the comment because HAVE_C_HEADER has the backslash. (Thanks
> > for catching the difference between HAVA_C_HEADER and HAVE_C_MACRO.) I think
> > another fix is needed to remove that backslash from HAVE_C_HEADER.  I've create
> > a one liner fix patch quickly [1]. It looks ok for blktests CI. I will revisit
> > it after Ming's patches get settled.
> > 
> > [1] https://github.com/osandov/blktests/pull/112/commits/dd5852e69abc3247d7b0ec4faf916a395378362d
> > 
> 
> Hello,
> 
> Sorry, I am not familiar with shell script. But **without** the backslash,
> I get this error:
> 
> $ make
> make -C src all
> make[1]: Entering directory '/home/alinux/workspace/blktests/src'
> Makefile:5: *** unterminated call to function 'shell': missing ')'.  Stop.
> make[1]: Leaving directory '/home/alinux/workspace/blktests/src'
> make: *** [Makefile:5: all] Error 2

I see... I googled and learned that make version 4.3 introduced this '# inside
macro' handling difference [2]. I guess your make has version older than 4.3,
isn't it?

[2] https://lwn.net/Articles/810071/

Per the the LWN article [2], the fix should be as follows. It works as expected
on my system with make version 4.3. Could you try it on your system?

diff --git a/src/Makefile b/src/Makefile
index 81c6541..322eb1c 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -1,8 +1,10 @@
-HAVE_C_HEADER = $(shell if echo "\#include <$(1)>" |		\
+H := \#
+
+HAVE_C_HEADER = $(shell if echo "$(H)include <$(1)>" |		\
 		$(CC) -E - > /dev/null 2>&1; then echo "$(2)";	\
 		else echo "$(3)"; fi)
 
-HAVE_C_MACRO = $(shell if echo "#include <$(1)>" |	\
+HAVE_C_MACRO = $(shell if echo -e "$(H)include <$(1)>" |	\
 		$(CC) -E - 2>&1 /dev/null | grep $(2) > /dev/null 2>&1; \
 		then echo 1;else echo 0; fi)
 


-- 
Shin'ichiro Kawasaki