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

[Ziyang Zhang <ZiyangZhang@linux.alibaba.com>]

On 2023/2/27 13:41, Shinichiro Kawasaki wrote:
> 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)

My make version is 4.2.1, and your fix works! Thanks, Shinichiro.

Regards,
Zhang