[PATCH v1] tests: getcwd.test

[Dmitry V. Levin]

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


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160322/9a5edaa4/attachment.bin>