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

[Dmitry Safonov]

2017-01-12 11:43 GMT+03:00 Vitaly Ostrosablin <vostrosablin at virtuozzo.com>:
>>2017-01-12 11:20 GMT+03:00 Dmitry Safonov <0x7f454c46 at gmail.com>:
>>> 2017-01-12 9:27 GMT+03:00 Vitaly Ostrosablin
>>> <vostrosablin at 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 at 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 at openvz.org
>>>> https://lists.openvz.org/mailman/listinfo/criu
>>
>>>
>>>
>>>
>>> --
>>>              Dmitry
>>
>>
>>
>>--
>>             Dmitry
>
>



-- 
             Dmitry