[1/2] test: replace cat in Makefiles with awk

Submitted by Vitaly Ostrosablin on Jan. 12, 2017, 6:27 a.m.

Details

Message ID 20170112062723.13352-1-vostrosablin@virtuozzo.com
State Superseded
Series "Series without cover letter"
Headers show

Commit Message

Vitaly Ostrosablin Jan. 12, 2017, 6:27 a.m.
cat *.pid works when we have only one running process at any moment.
That's because pidfiles contain no newlines and cat will just
concatenate content of all files present in directory. So, e.g., if we
have pidfiles:

31338
31359
31880
31884
31889

cat will build following string from those pidfiles:

3133831359318803188431889

Obviously, kill would fail to send signals to processes, because it's
now a single big number, which cannot be unambigously split back in
general case.

That's where awk comes in. We don't need to modify C part of tests to
print newlines at end of file. We just order awk to print content of
each file, which adds a newline at end - problem solved, kill can once
again parse, which PIDs it get and send proper signals to them. Also,
should be completely safe for zdtm.py, because it doesn't use Makefiles
and sends signals on it's own.
---
 test/zdtm/static/Makefile     | 4 ++--
 test/zdtm/transition/Makefile | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
index af1254f5..8fe9d953 100644
--- a/test/zdtm/static/Makefile
+++ b/test/zdtm/static/Makefile
@@ -338,7 +338,7 @@  start:	$(PID) $(STATE)
 check_start:	$(PID:%.pid=%.is_running)
 
 stop:	$(STATE_OUT)
-	-kill -TERM `cat *.pid`
+	-kill -TERM `awk '{print}' *.pid`
 
 WAIT_TIME=240
 %.stop:	%.pid %
@@ -356,7 +356,7 @@  WAIT_TIME=240
 wait_stop:
 		i=0; \
 		while [ $$i -lt $(WAIT_TIME) ] ; do \
-		kill -0 `cat *.pid 2>/dev/null` 2>/dev/null || break; \
+		kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; \
 		sleep 1; \
 		i=`expr $$i + 1`; \
 	done
diff --git a/test/zdtm/transition/Makefile b/test/zdtm/transition/Makefile
index 7ddb2384..dfc10ef5 100644
--- a/test/zdtm/transition/Makefile
+++ b/test/zdtm/transition/Makefile
@@ -65,12 +65,12 @@  start:	$(PID)
 check_start:	$(PID:%.pid=%.is_running)
 
 stop:
-	-kill -TERM `cat *.pid`
+	-kill -TERM `awk '{print}' *.pid`
 
 WAIT_TIME=10
 wait_stop:
 	-for i in `seq 1 $(WAIT_TIME)`; do \
-		kill -0 `cat *.pid 2>/dev/null` 2>/dev/null || break; \
+		kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; \
 		sleep 1; \
 	done
 

Comments

Dmitry Safonov Jan. 12, 2017, 8:20 a.m.
2017-01-12 9:27 GMT+03:00 Vitaly Ostrosablin <vostrosablin@virtuozzo.com>:
> cat *.pid works when we have only one running process at any moment.
> That's because pidfiles contain no newlines and cat will just
> concatenate content of all files present in directory. So, e.g., if we
> have pidfiles:
>
> 31338
> 31359
> 31880
> 31884
> 31889
>
> cat will build following string from those pidfiles:
>
> 3133831359318803188431889

I don't mind the patch, but your explanation looks obiviously wrong:
[test]$ echo 123 > 1.txt
[test]$ echo 456 > 2.txt
[test]$ cat *.txt
123
456
[test]$ cat Makefile
all:
    cat *.txt
[test]$ make
cat *.txt
123
456

Is it something that depends on environment, like ${IFS} in bash?
Check, please.

>
> Obviously, kill would fail to send signals to processes, because it's
> now a single big number, which cannot be unambigously split back in
> general case.
>
> That's where awk comes in. We don't need to modify C part of tests to
> print newlines at end of file. We just order awk to print content of
> each file, which adds a newline at end - problem solved, kill can once
> again parse, which PIDs it get and send proper signals to them. Also,
> should be completely safe for zdtm.py, because it doesn't use Makefiles
> and sends signals on it's own.
> ---
>  test/zdtm/static/Makefile     | 4 ++--
>  test/zdtm/transition/Makefile | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
> index af1254f5..8fe9d953 100644
> --- a/test/zdtm/static/Makefile
> +++ b/test/zdtm/static/Makefile
> @@ -338,7 +338,7 @@ start:      $(PID) $(STATE)
>  check_start:   $(PID:%.pid=%.is_running)
>
>  stop:  $(STATE_OUT)
> -       -kill -TERM `cat *.pid`
> +       -kill -TERM `awk '{print}' *.pid`
>
>  WAIT_TIME=240
>  %.stop:        %.pid %
> @@ -356,7 +356,7 @@ WAIT_TIME=240
>  wait_stop:
>                 i=0; \
>                 while [ $$i -lt $(WAIT_TIME) ] ; do \
> -               kill -0 `cat *.pid 2>/dev/null` 2>/dev/null || break; \
> +               kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; \
>                 sleep 1; \
>                 i=`expr $$i + 1`; \
>         done
> diff --git a/test/zdtm/transition/Makefile b/test/zdtm/transition/Makefile
> index 7ddb2384..dfc10ef5 100644
> --- a/test/zdtm/transition/Makefile
> +++ b/test/zdtm/transition/Makefile
> @@ -65,12 +65,12 @@ start:      $(PID)
>  check_start:   $(PID:%.pid=%.is_running)
>
>  stop:
> -       -kill -TERM `cat *.pid`
> +       -kill -TERM `awk '{print}' *.pid`
>
>  WAIT_TIME=10
>  wait_stop:
>         -for i in `seq 1 $(WAIT_TIME)`; do \
> -               kill -0 `cat *.pid 2>/dev/null` 2>/dev/null || break; \
> +               kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; \
>                 sleep 1; \
>         done
>
> --
> 2.11.0
>
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Dmitry Safonov Jan. 12, 2017, 8:24 a.m.
2017-01-12 11:20 GMT+03:00 Dmitry Safonov <0x7f454c46@gmail.com>:
> 2017-01-12 9:27 GMT+03:00 Vitaly Ostrosablin <vostrosablin@virtuozzo.com>:
>> cat *.pid works when we have only one running process at any moment.
>> That's because pidfiles contain no newlines and cat will just
>> concatenate content of all files present in directory. So, e.g., if we
>> have pidfiles:
>>
>> 31338
>> 31359
>> 31880
>> 31884
>> 31889
>>
>> cat will build following string from those pidfiles:
>>
>> 3133831359318803188431889
>
> I don't mind the patch, but your explanation looks obiviously wrong:
> [test]$ echo 123 > 1.txt
> [test]$ echo 456 > 2.txt
> [test]$ cat *.txt
> 123
> 456
> [test]$ cat Makefile
> all:
>     cat *.txt
> [test]$ make
> cat *.txt
> 123
> 456

[test]$ cat Makefile
all:
    echo `cat *.txt`
[test]$ make
echo `cat *.txt`
123 456
[test]$

>
> Is it something that depends on environment, like ${IFS} in bash?
> Check, please.
>
>>
>> Obviously, kill would fail to send signals to processes, because it's
>> now a single big number, which cannot be unambigously split back in
>> general case.
>>
>> That's where awk comes in. We don't need to modify C part of tests to
>> print newlines at end of file. We just order awk to print content of
>> each file, which adds a newline at end - problem solved, kill can once
>> again parse, which PIDs it get and send proper signals to them. Also,
>> should be completely safe for zdtm.py, because it doesn't use Makefiles
>> and sends signals on it's own.
>> ---
>>  test/zdtm/static/Makefile     | 4 ++--
>>  test/zdtm/transition/Makefile | 4 ++--
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
>> index af1254f5..8fe9d953 100644
>> --- a/test/zdtm/static/Makefile
>> +++ b/test/zdtm/static/Makefile
>> @@ -338,7 +338,7 @@ start:      $(PID) $(STATE)
>>  check_start:   $(PID:%.pid=%.is_running)
>>
>>  stop:  $(STATE_OUT)
>> -       -kill -TERM `cat *.pid`
>> +       -kill -TERM `awk '{print}' *.pid`
>>
>>  WAIT_TIME=240
>>  %.stop:        %.pid %
>> @@ -356,7 +356,7 @@ WAIT_TIME=240
>>  wait_stop:
>>                 i=0; \
>>                 while [ $$i -lt $(WAIT_TIME) ] ; do \
>> -               kill -0 `cat *.pid 2>/dev/null` 2>/dev/null || break; \
>> +               kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; \
>>                 sleep 1; \
>>                 i=`expr $$i + 1`; \
>>         done
>> diff --git a/test/zdtm/transition/Makefile b/test/zdtm/transition/Makefile
>> index 7ddb2384..dfc10ef5 100644
>> --- a/test/zdtm/transition/Makefile
>> +++ b/test/zdtm/transition/Makefile
>> @@ -65,12 +65,12 @@ start:      $(PID)
>>  check_start:   $(PID:%.pid=%.is_running)
>>
>>  stop:
>> -       -kill -TERM `cat *.pid`
>> +       -kill -TERM `awk '{print}' *.pid`
>>
>>  WAIT_TIME=10
>>  wait_stop:
>>         -for i in `seq 1 $(WAIT_TIME)`; do \
>> -               kill -0 `cat *.pid 2>/dev/null` 2>/dev/null || break; \
>> +               kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; \
>>                 sleep 1; \
>>         done
>>
>> --
>> 2.11.0
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu
>
>
>
> --
>              Dmitry
Vitaly Ostrosablin Jan. 12, 2017, 8:43 a.m.
>2017-01-12 11:20 GMT+03:00 Dmitry Safonov <0x7f454c46@gmail.com>:

>> 2017-01-12 9:27 GMT+03:00 Vitaly Ostrosablin <vostrosablin@virtuozzo.com>:

>>> cat *.pid works when we have only one running process at any moment.

>>> That's because pidfiles contain no newlines and cat will just

>>> concatenate content of all files present in directory. So, e.g., if we

>>> have pidfiles:

>>>

>>> 31338

>>> 31359

>>> 31880

>>> 31884

>>> 31889

>>>

>>> cat will build following string from those pidfiles:

>>>

>>> 3133831359318803188431889

>>

>> I don't mind the patch, but your explanation looks obiviously wrong:

>> [test]$ echo 123 > 1.txt

>> [test]$ echo 456 > 2.txt

>> [test]$ cat *.txt

>> 123

>> 456

>> [test]$ cat Makefile

>> all:

>>     cat *.txt

>> [test]$ make

>> cat *.txt

>> 123

>> 456

>

>[test]$ cat Makefile

>all:

>    echo `cat *.txt`

>[test]$ make

>echo `cat *.txt`

>123 456

>[test]$

>



It doesn't look really wrong. What cat does is just a concatenation

of files content and doesn't modify it in any way. What you put into

files is what you get. So, concatenating '123\n' and '456\n' gives


123

456


Now let's do same with concatenation of '123' and '456'. The result

is clearly:


123456


cat doesn't append it's own newlines. Now let's use echo with no-newline

option:


 echo -n 123 > 1.txt
 echo -n 456 > 2.txt
 cat *.txt
123456%

(Percent sign at end in zsh means that output ended without newline being
printed.)

Here's how we write the pidfile in zdtm lib/test.c:

if (dprintf(fd, "%d", pid) == -1) {
fprintf(stderr, "Can't write in the file %s: %m\n", tmp);
goto err_c;
}

It just prints %d, i.e. pid only, without newline at end. So, if we were to cat
all pidfiles, we'll just get a single huge undelimited number.

>>

>> Is it something that depends on environment, like ${IFS} in bash?

>> Check, please.

>>

>>>

>>> Obviously, kill would fail to send signals to processes, because it's

>>> now a single big number, which cannot be unambigously split back in

>>> general case.

>>>

>>> That's where awk comes in. We don't need to modify C part of tests to

>>> print newlines at end of file. We just order awk to print content of

>>> each file, which adds a newline at end - problem solved, kill can once

>>> again parse, which PIDs it get and send proper signals to them. Also,

>>> should be completely safe for zdtm.py, because it doesn't use Makefiles

>>> and sends signals on it's own.

>>> ---

>>>  test/zdtm/static/Makefile     | 4 ++--

>>>  test/zdtm/transition/Makefile | 4 ++--

>>>  2 files changed, 4 insertions(+), 4 deletions(-)

>>>

>>> diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile

>>> index af1254f5..8fe9d953 100644

>>> --- a/test/zdtm/static/Makefile

>>> +++ b/test/zdtm/static/Makefile

>>> @@ -338,7 +338,7 @@ start:      $(PID) $(STATE)

>>>  check_start:   $(PID:%.pid=%.is_running)

>>>

>>>  stop:  $(STATE_OUT)

>>> -       -kill -TERM `cat *.pid`

>>> +       -kill -TERM `awk '{print}' *.pid`

>>>

>>>  WAIT_TIME=240

>>>  %.stop:        %.pid %

>>> @@ -356,7 +356,7 @@ WAIT_TIME=240

>>>  wait_stop:

>>>                 i=0; >>                 while [ $$i -lt $(WAIT_TIME) ] ; do >> -               kill -0 `cat *.pid 2>/dev/null` 2>

/dev/null || break; >> +               kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; >>                 sleep 1; >>
                i=`expr $$i + 1`; >>         done
>>> diff --git a/test/zdtm/transition/Makefile b/test/zdtm/transition/Makefile

>>> index 7ddb2384..dfc10ef5 100644

>>> --- a/test/zdtm/transition/Makefile

>>> +++ b/test/zdtm/transition/Makefile

>>> @@ -65,12 +65,12 @@ start:      $(PID)

>>>  check_start:   $(PID:%.pid=%.is_running)

>>>

>>>  stop:

>>> -       -kill -TERM `cat *.pid`

>>> +       -kill -TERM `awk '{print}' *.pid`

>>>

>>>  WAIT_TIME=10

>>>  wait_stop:

>>>         -for i in `seq 1 $(WAIT_TIME)`; do >> -               kill -0 `cat *.pid 2>/dev/null` 2>/dev/null || break; >> +

     kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; >>                 sleep 1; >>         done
>>>

>>> --

>>> 2.11.0

>>>

>>> _______________________________________________

>>> CRIU mailing list

>>> CRIU@openvz.org

>>> https://lists.openvz.org/mailman/listinfo/criu

>

>>

>>

>>

>> --

>>              Dmitry

>

>

>

>--

>             Dmitry
Dmitry Safonov Jan. 12, 2017, 8:48 a.m.
2017-01-12 11:43 GMT+03:00 Vitaly Ostrosablin <vostrosablin@virtuozzo.com>:
>>2017-01-12 11:20 GMT+03:00 Dmitry Safonov <0x7f454c46@gmail.com>:
>>> 2017-01-12 9:27 GMT+03:00 Vitaly Ostrosablin
>>> <vostrosablin@virtuozzo.com>:
>>>> cat *.pid works when we have only one running process at any moment.
>>>> That's because pidfiles contain no newlines and cat will just
>>>> concatenate content of all files present in directory. So, e.g., if we
>>>> have pidfiles:
>>>>
>>>> 31338
>>>> 31359
>>>> 31880
>>>> 31884
>>>> 31889
>>>>
>>>> cat will build following string from those pidfiles:
>>>>
>>>> 3133831359318803188431889
>>>
>>> I don't mind the patch, but your explanation looks obiviously wrong:
>>> [test]$ echo 123 > 1.txt
>>> [test]$ echo 456 > 2.txt
>>> [test]$ cat *.txt
>>> 123
>>> 456
>>> [test]$ cat Makefile
>>> all:
>>>     cat *.txt
>>> [test]$ make
>>> cat *.txt
>>> 123
>>> 456
>>
>>[test]$ cat Makefile
>>all:
>>    echo `cat *.txt`
>>[test]$ make
>>echo `cat *.txt`
>>123 456
>>[test]$
>>
>
>
> It doesn't look really wrong. What cat does is just a concatenation
>
> of files content and doesn't modify it in any way. What you put into
>
> files is what you get. So, concatenating '123\n' and '456\n' gives
>
>
> 123
>
> 456
>
>
> Now let's do same with concatenation of '123' and '456'. The result
>
> is clearly:
>
>
> 123456
>
>
> cat doesn't append it's own newlines. Now let's use echo with no-newline
>
> option:
>
>
>  echo -n 123 > 1.txt
>  echo -n 456 > 2.txt
>  cat *.txt
> 123456%

Yep, it's my wrong test.
Checked the newline with vim in *.txt and it doesn't show a new line %)

Ok, no objections from me, thanks:
Reviewed-by: Dmitry Safonov <dsafonov@virtuozzo.com>

>
> (Percent sign at end in zsh means that output ended without newline being
> printed.)
>
> Here's how we write the pidfile in zdtm lib/test.c:
>
> if (dprintf(fd, "%d", pid) == -1) {
> fprintf(stderr, "Can't write in the file %s: %m\n", tmp);
> goto err_c;
> }
>
> It just prints %d, i.e. pid only, without newline at end. So, if we were to
> cat
> all pidfiles, we'll just get a single huge undelimited number.
>
>
>>>
>>> Is it something that depends on environment, like ${IFS} in bash?
>>> Check, please.
>>>
>>>>
>>>> Obviously, kill would fail to send signals to processes, because it's
>>>> now a single big number, which cannot be unambigously split back in
>>>> general case.
>>>>
>>>> That's where awk comes in. We don't need to modify C part of tests to
>>>> print newlines at end of file. We just order awk to print content of
>>>> each file, which adds a newline at end - problem solved, kill can once
>>>> again parse, which PIDs it get and send proper signals to them. Also,
>>>> should be completely safe for zdtm.py, because it doesn't use Makefiles
>>>> and sends signals on it's own.
>>>> ---
>>>>  test/zdtm/static/Makefile     | 4 ++--
>>>>  test/zdtm/transition/Makefile | 4 ++--
>>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile
>>>> index af1254f5..8fe9d953 100644
>>>> --- a/test/zdtm/static/Makefile
>>>> +++ b/test/zdtm/static/Makefile
>>>> @@ -338,7 +338,7 @@ start:      $(PID) $(STATE)
>>>>  check_start:   $(PID:%.pid=%.is_running)
>>>>
>>>>  stop:  $(STATE_OUT)
>>>> -       -kill -TERM `cat *.pid`
>>>> +       -kill -TERM `awk '{print}' *.pid`
>>>>
>>>>  WAIT_TIME=240
>>>>  %.stop:        %.pid %
>>>> @@ -356,7 +356,7 @@ WAIT_TIME=240
>>>>  wait_stop:
>>>>                 i=0; >>                 while [ $$i -lt $(WAIT_TIME) ] ;
>>>> do >> -               kill -0 `cat *.pid 2>/dev/null` 2>
> /dev/null || break; >> +               kill -0 `awk '{print}' *.pid
> 2>/dev/null` 2>/dev/null || break; >>                 sleep 1; >>
>                 i=`expr $$i + 1`; >>         done
>>>> diff --git a/test/zdtm/transition/Makefile
>>>> b/test/zdtm/transition/Makefile
>>>> index 7ddb2384..dfc10ef5 100644
>>>> --- a/test/zdtm/transition/Makefile
>>>> +++ b/test/zdtm/transition/Makefile
>>>> @@ -65,12 +65,12 @@ start:      $(PID)
>>>>  check_start:   $(PID:%.pid=%.is_running)
>>>>
>>>>  stop:
>>>> -       -kill -TERM `cat *.pid`
>>>> +       -kill -TERM `awk '{print}' *.pid`
>>>>
>>>>  WAIT_TIME=10
>>>>  wait_stop:
>>>>         -for i in `seq 1 $(WAIT_TIME)`; do >> -               kill -0
>>>> `cat *.pid 2>/dev/null` 2>/dev/null || break; >> +
>      kill -0 `awk '{print}' *.pid 2>/dev/null` 2>/dev/null || break; >>
> sleep 1; >>         done
>>>>
>>>> --
>>>> 2.11.0
>>>>
>>>> _______________________________________________
>>>> CRIU mailing list
>>>> CRIU@openvz.org
>>>> https://lists.openvz.org/mailman/listinfo/criu
>>
>>>
>>>
>>>
>>> --
>>>              Dmitry
>>
>>
>>
>>--
>>             Dmitry
>
>
Cyrill Gorcunov Jan. 12, 2017, 8:53 a.m.
On Thu, Jan 12, 2017 at 09:27:22AM +0300, Vitaly Ostrosablin wrote:
> cat *.pid works when we have only one running process at any moment.
> That's because pidfiles contain no newlines and cat will just
> concatenate content of all files present in directory. So, e.g., if we
> have pidfiles:

SOB is missed again, please use proper patch format next time.
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
for both