[PATCH v1] tests: getcwd.test

[Jay Joshi]

On Tue, Mar 22, 2016 at 5:15 AM, Dmitry V. Levin <ldv at altlinux.org> wrote:
> On Mon, Mar 21, 2016 at 03:17:19PM +0530, Jay Joshi wrote:
>> Subject: [PATCH] tests: add getcwd.test, add print_quoted_string function to
>>  libtests
>>
>> * tests/tests.h (print_quoted_string): New prototype.
>> * tests/print_quoted_string.c: New file.
>> * tests/Makefile.am (libtests_a_SOURCES): Add it.
>
> Looks like it's better to split this patch.  The first one would just add
> print_quoted_string, ...
>
>> * tests/getcwd.c: New file.
>> * tests/getcwd.test: New test.
>> * tests/.gitignore: Add getcwd.
>> * tests/Makefile.am (check_PROGRAMS): Likewise.
>> (TESTS): Add getcwd.test.
>
> ... and the second would add this test.
>
> Besides the split and several minor comments below, the patch seems ready.
>
>> --- /dev/null
>> +++ b/tests/getcwd.test
>> @@ -0,0 +1,13 @@
>> +#!/bin/sh
>> +
>> +# Check getcwd syscall decoding.
>> +
>> +. "${srcdir=.}/init.sh"
>> +
>> +run_prog > /dev/null
>> +OUT="$LOG.out"
>> +run_strace -egetcwd -a18 $args > $OUT
>> +match_diff "$LOG" "$OUT"
>> +rm -f "$OUT"
>
> Let's keep everything quoted the same way.
>
>> +
>> +exit 0
>
> You don't really need this.  I know I've added a lot of them myself,
> but they are not needed in new files anyway.
>
>> --- /dev/null
>> +++ b/tests/print_quoted_string.c
>> @@ -0,0 +1,67 @@
>> +#include "tests.h"
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +
>> +/* Modified from string_quote() from util.c.
>
> It's "Based on" rather than "Modified from".
>
> Also, our preferred boxed comment style is
> /*
>  * Lines of
>  * Text.
>  */
>
>> +                                             /* Print \[[o]o]o */
>> +                                             if ((c >> 3) != 0) {
>> +                                                     if ((c >> 6) != 0)
>> +                                                             putchar(c3);
>> +                                                     putchar(c2);
>> +                                             }
>
> Isn't this more readable:
>
>         if (c3 != '0' || c2 != '0') {
>                 if (c3 != '0')
>                         putchar(c3);
>                 putchar(c2);
>         }
>
>> +/* Print string in escaped format. */
>> +void print_quoted_string(const char *str);
>> +
>
> escaped vs quoted
>

Patch is attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: a.patch
Type: text/x-patch
Size: 5189 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160322/9ca03adf/attachment.bin>