Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 22 Jun 2015 19:26:55 +0800
From: Kai Zhao <loverszhao@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: Coding Style

I have created a john-1.8.0 mirror to test the result of indent.

https://github.com/loverszhaokai/john_core_coding_style/commit/078aa5d4465c4b2bb5710ead5993dde9532db52c

command: indent -kr -i8 -ts8 -nlp -ci4 -nbbo -ncs -l80 -lc80 -bad -il0
 src/*.[ch]
indent version: GNU indent 2.2.11

There are about 9 breakages by indent. I created a demo for breakages.
The attached files origin.c is the origin file which are the current coding
style of john core. And the result.c is the formatted result by the indent
command. The diff.txt is the output of 'diff origin.c result.c'. Maybe there
are also other breakages, but I think the 9 breakages are the most
concerned.

You can see that indent does help and does break. I think manual
review is a must if we use any source code format tools, since any
tools maybe make breakages.

Should we use indent ? Or should we patch indent ?

I think some of breaks such as the case below (b2) can be supported
by patching.

if (condition)
if (condition) {
        do_something();
        do_someting();
}

Thanks for your advice.


Thanks,

Kai

[ CONTENT OF TYPE text/html SKIPPED ]

2c2
< static void (*b1)(void);
---
> static void (*b1) (void);
8,11c8,11
< 	if (condition) {
< 		do_something();
< 		do_someting();
< 	}
---
> 		if (condition) {
> 			do_something();
> 			do_someting();
> 		}
17,20c17,20
< 	for (;;) {
< 		do_someting();
< 		do_someting();
< 	}
---
> 		for (;;) {
> 			do_someting();
> 			do_someting();
> 		}
26c26
< 	ret = fff() ? 1 : 0;
---
> 	ret = fff()? 1 : 0;
31c31
< void (*b4) (struct db_main *db, char *line);
---
> void (*b4) (struct db_main * db, char *line);
43,45c43,46
< 		if ((unsigned ARCH_WORD)binary[word + 2] !=
< 		    (buffer[index].aligned.binary[word] & TOTAL_BINARY_MASK))
< 			return;
---
> 			if ((unsigned ARCH_WORD)binary[word + 2] !=
> 			    (buffer[index].aligned.
> 				binary[word] & TOTAL_BINARY_MASK))
> 				return;
52,73c53,70
< 		FORMAT_LABEL,
< 		FMT_CASE | FMT_8_BIT,
< 		tests
< 	}, {
< 		init,
< 		fmt_default_source,
< 		{
< 			binary_hash_0,
< 			binary_hash_2,
< 			NULL,
< 			NULL
< 		},
< 		fmt_default_salt_hash,
< 		crypt_all,
< 		{
< 			get_hash_0,
< 			NULL,
< 			NULL
< 		},
< 		cmp_all,
< 		cmp_exact
< 	}
---
> 		    FORMAT_LABEL,
> 		    FMT_CASE | FMT_8_BIT,
> 	    tests}, {
> 		    init,
> 		    fmt_default_source,
> 		    {
> 				binary_hash_0,
> 				binary_hash_2,
> 				NULL,
> 			NULL},
> 		    fmt_default_salt_hash,
> 		    crypt_all,
> 		    {
> 				get_hash_0,
> 				NULL,
> 			NULL},
> 		    cmp_all,
> 	    cmp_exact}
79,81c76,78
< 		"0123456789abcdefghijklmnopqrstuvwxyz"
< 		"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
< 		"chars after 72 are ignored"},
---
> 	    "0123456789abcdefghijklmnopqrstuvwxyz"
> 		    "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
> 		    "chars after 72 are ignored"},
88c85
< 	b = (People *)&id;
---
> 	b = (People *) & id;
103,106c100,103
< 	do {
< 		do_something();
< 	} while (true);
< }
---
> 		do {
> 			do_something();
> 		} while (true);
> 	}

// b1
static void (*b1)(void);


void b2()
{
	if (condition)
	if (condition) {
		do_something();
		do_someting();
	}

	if (condition) {
		do_someting();
		do_someting();
	} else
	for (;;) {
		do_someting();
		do_someting();
	}
}


void b3()
{
	ret = fff() ? 1 : 0;
}


// b4
void (*b4) (struct db_main *db, char *line);

void b4(struct db_main *db, char *line)
{
	struct db_main *db = NULL;
}


void b5()
{
	if (condition())
		if (condition())
		if ((unsigned ARCH_WORD)binary[word + 2] !=
		    (buffer[index].aligned.binary[word] & TOTAL_BINARY_MASK))
			return;
	return;
}


struct fmt_main b6 = {
	{
		FORMAT_LABEL,
		FMT_CASE | FMT_8_BIT,
		tests
	}, {
		init,
		fmt_default_source,
		{
			binary_hash_0,
			binary_hash_2,
			NULL,
			NULL
		},
		fmt_default_salt_hash,
		crypt_all,
		{
			get_hash_0,
			NULL,
			NULL
		},
		cmp_all,
		cmp_exact
	}
};


static struct fmt_tests b7[] = {
	{"$2a$05$abcdefghijklmnopqrstuu5s2v8.iXieOjg/.AySBTTZIIVFJeBui",
		"0123456789abcdefghijklmnopqrstuvwxyz"
		"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
		"chars after 72 are ignored"},
	{NULL}
};


void b8()
{
	b = (People *)&id;
}


void b9()
{
#if CON
	if (condition()) {
#endif
		do_something();
#if CON
	}
#else
#endif

	do {
		do_something();
	} while (true);
}

// b1
static void (*b1) (void);


void b2()
{
	if (condition)
		if (condition) {
			do_something();
			do_someting();
		}

	if (condition) {
		do_someting();
		do_someting();
	} else
		for (;;) {
			do_someting();
			do_someting();
		}
}


void b3()
{
	ret = fff()? 1 : 0;
}


// b4
void (*b4) (struct db_main * db, char *line);

void b4(struct db_main *db, char *line)
{
	struct db_main *db = NULL;
}


void b5()
{
	if (condition())
		if (condition())
			if ((unsigned ARCH_WORD)binary[word + 2] !=
			    (buffer[index].aligned.
				binary[word] & TOTAL_BINARY_MASK))
				return;
	return;
}


struct fmt_main b6 = {
	{
		    FORMAT_LABEL,
		    FMT_CASE | FMT_8_BIT,
	    tests}, {
		    init,
		    fmt_default_source,
		    {
				binary_hash_0,
				binary_hash_2,
				NULL,
			NULL},
		    fmt_default_salt_hash,
		    crypt_all,
		    {
				get_hash_0,
				NULL,
			NULL},
		    cmp_all,
	    cmp_exact}
};


static struct fmt_tests b7[] = {
	{"$2a$05$abcdefghijklmnopqrstuu5s2v8.iXieOjg/.AySBTTZIIVFJeBui",
	    "0123456789abcdefghijklmnopqrstuvwxyz"
		    "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
		    "chars after 72 are ignored"},
	{NULL}
};


void b8()
{
	b = (People *) & id;
}


void b9()
{
#if CON
	if (condition()) {
#endif
		do_something();
#if CON
	}
#else
#endif

		do {
			do_something();
		} while (true);
	}

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ