Compile fails with gcc 6.1.1

[lrvick]

make -C client all
make[1]: Entering directory '/home/lrvick/Sources/proxmark3/client'
Compiling liblua, using platform linux
cd ../liblua && make linux
make[2]: Entering directory '/home/lrvick/Sources/proxmark3/liblua'
make all SYSCFLAGS="-DLUA_USE_LINUX" SYSLIBS="-Wl,-E -ldl -lreadline"
make[3]: Entering directory '/home/lrvick/Sources/proxmark3/liblua'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/lrvick/Sources/proxmark3/liblua'
make[2]: Leaving directory '/home/lrvick/Sources/proxmark3/liblua'
make[1]: Leaving directory '/home/lrvick/Sources/proxmark3/client'
make -C bootrom all
make[1]: Entering directory '/home/lrvick/Sources/proxmark3/bootrom'
perl ../tools/mkversion.pl .. > version.c || cp ../common/default_version.c version.c 
arm-none-eabi-gcc -c -I../include -I../common -Wall -Werror -pedantic -std=c99 -Os -I. -mthumb -mthumb-interwork -o obj/version.o version.c 
arm-none-eabi-gcc -c -I../include -I../common -Wall -Werror -pedantic -std=c99 -Os -I. -mthumb -mthumb-interwork -o obj/bootrom.o bootrom.c 
bootrom.c: In function 'flash_mode':
bootrom.c:206:3: error: this 'for' clause does not guard... [-Werror=misleading-indentatio ]
   for (volatile size_t i=0; i<0x100000; i++);
   ^~~
bootrom.c:208:2: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
  for(;;) {
  ^~~
cc1: all warnings being treated as errors
make[1]: *** [../common/Makefile.common:81: obj/bootrom.o] Error 1
make[1]: Leaving directory '/home/lrvick/Sources/proxmark3/bootrom'
make: *** [Makefile:8: bootrom/all] Error 2

[lrvick]

Went down the rabbit trail trying to fix indentation to make gcc happy... and it is a mess, so I will leave that to someone else.

Workaround for the moment, disable -Werror:

diff --git a/common/Makefile.common b/common/Makefile.common
index 7c94e04..1bc6413 100644
--- a/common/Makefile.common
+++ b/common/Makefile.common
@@ -67,7 +67,7 @@ VPATH = . ../common ../fpga ../zlib

 INCLUDES = ../include/proxmark3.h ../include/at91sam7s512.h ../include/config_gpio.h ../include/usb_cmd.h $(APP_INCLUDES)

-CFLAGS =  -c $(INCLUDE) -Wall -Werror -pedantic -std=c99 -Os $(APP_CFLAGS)
+CFLAGS =  -c $(INCLUDE) -Wall -pedantic -std=c99 -Os $(APP_CFLAGS)
 LDFLAGS = -nostartfiles -nodefaultlibs -Wl,-gc-sections -n

 LIBS = -lgcc

[ikarus23]

Bump.

[ikarus23]

As @lrvick said, the issue is with all the mixed up indentation styles within this project. The new gcc don't likes this and is issuing a warning.

And @lrvick is right. The indentation is really messed up. Spaces, tabs, different indent width of 2, 4, or whatever. All over the files.

The whole project needs refactoring. There should be a simple indent rule across the project like two spaces. And most of all: who has the time and is willing to do this? At least for all the .c and .h files.

@iceman1001 @holiman @marshmellow42 @pwpiwi & every other active developer: Is it easily possible for you to keep such an indent rule as two spaces (e.g. change the settings of your favorite editor)?

[ikarus23]

There is a tool for formatting c code by the name of indent. My suggestion is to run this on all proxmark .c/.h files with indent -linux -l120 -i2 -nut. This will format all the code like it is done in the linux kernel, except that the line length is 120 character and the indents are 2 spaces.

I tried it. After this, all indent warnings/errors are gone and the project builds with the new gcc.

[pwpiwi]

This warning is about misleading indentation, not about subjective coding style. Don't we all use tabs instead of spaces anyway?

Instead of removing-Werror I propose to add -Wno-error=misleading-indentation

[iceman1001]

The first line in our HACKING.TXT describes it quite well.

"Coding styles are like assholes, everyone has one and no one likes anyone elses." / Eric Warmenhoven

and then the same document starts with:

Use tabs for indentation, but use spaces for alignment:

Still I do like the idea of having a unified way, I'm already sensing that a energy sucking discussion about 2,4,8 spaces as indentation is not any of my interest. There is a reason for me having done whatever in my fork and only some of it finds its way back in to PM3 master.
If that cool "indent" software works with setting tabs, I'll propably apply it on my fork and think no more of it.

I'll rather have ppl contribution to the code functionality than structual ideas at this point in time.
if @pwpiwi 's suggested removal of the warning works, then I will support that one.

[ikarus23]

You are right @iceman1001. This discussion will be very frustrating and energy sucking, most of all because I prefer spaces over tabs :). Also, it is very hard to enforce a coding style to a project at that stage (age).

Therefore, although I don't like the proxmark code (tabs/spaces/line feeds/etc), I will also stick with @pwpiwi suggestion. This seams to be the easy way out.

[iceman1001]

I dl the intend software pretty cool. It makes the source code quite unified with spaces between. I used the -kr option and liked what I saw. Did the misstake to apply to . which messed up my scripts. so just *.c and *.h I'll apply that stuff in my fork. step byt step.

[ikarus23]

Hehe, yes, .c/.h files should be enough. Also, I don't think a refactoring of the liblua code is needed.

For now I pushed @pwpiwi suggestion to the master. Everything should be fine now for gcc 6+ users.
But yes, the intend tool is really cool and makes the code really readable. ;)

[ikarus23]

Reopened. As @iceman1001 pointed out at fd4f21b, @pwpiwi solution does not work for people without gcc 6+.

[ikarus23]

I fixed all "misleading-indentation" warnings. That should work for everyone.

[pwpiwi]

Definitely the best solution. I didn't even think about this because of

Went down the rabbit trail trying to fix indentation to make gcc happy... and it is a mess, so I will leave that to someone else.

😁