[PATCH v1] tests: getcwd.test

Dmitry V. Levin ldv at altlinux.org
Sun Mar 20 23:13:16 UTC 2016


On Sun, Mar 20, 2016 at 11:12:08AM +0530, Jay Joshi wrote:
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -49,6 +49,7 @@ libtests_a_SOURCES = \
>  	tail_alloc.c \
>  	tests.h \
>  	tprintf.c \
> +	print_quoted_string.c \
>  	# end of libtests_a_SOURCES

Please keep the list sorted.

> --- /dev/null
> +++ b/tests/print_quoted_string.c
> @@ -0,0 +1,61 @@
> +#include "tests.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +/* Modified from string_quote() from util.c.
> + * Assumes str is NUL-terminated.
> + */
> +
> +void
> +print_quoted_string(const char *str)
> +{
> +	unsigned int i=0;
> +	int c;

Shouldn't "c" have type "unsigned char", or, alternatively,
shouldn't "str" be cast to "const unsigned char *", to avoid sign
extension?

> +
> +	while (c = str[i++]) {

Wouldn't it be better without "i" iterator at all?  e.g.
       	while ((c = *(str++))) {

> +			default:
> +				if (c >= ' ' && c <= 0x7e)
> +					putchar(c);
> +				else {
> +					printf("\\");

Wouldn't putchar(c) in cases like this look simpler? 

> +					if (str[i + 1] >= '0' && str[i + 1] <= '9') {

This check is not correct because "i" already points to the next
character.


-- 
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/20160321/4a9818f2/attachment.bin>


More information about the Strace-devel mailing list