Get rid of for (uint8_t i = blah stuff in serial.c

This forum is dedicated to software development related to MultiWii.
It is not the right place to submit a setup problem.
Software download
Post Reply
timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Get rid of for (uint8_t i = blah stuff in serial.c

Post by timecop »

Guys, I realize that you like to type same stuff over and over but,

in evaluateCommand() there's like 20 occurences of the thing in /subject without any reason.
Protip:

void evaluateCommand(void)
{
uint8_t i; // do it once, here

...

for (i = 0; i < .... );
}

Thanks! My compiler thanks you too.

Tommie
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by Tommie »

Doesn't make any difference here.

With variables scoped to the for-loop:

Code: Select all

stefan@exa:~/git/multiwii-firmware [tupper] $ avr-size build/*.hex
   text      data       bss       dec       hex   filename
      0     26982         0     26982      6966   build/MultiWii.hex


With one variable defined at the start of evaluate_command:

Code: Select all

stefan@exa:~/git/multiwii-firmware [tupper…] $ avr-size build/*.hex
   text      data       bss       dec       hex   filename
      0     26982         0     26982      6966   build/MultiWii.hex


Since only one branch of the switch statement will be executed anyway, the multiple declarations inside the varous for loops will probably be optimized away by the compiler.

User avatar
EOSBandi
Posts: 802
Joined: Sun Jun 19, 2011 11:32 am
Location: Budapest, Hungary
Contact:

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by EOSBandi »

Local, non static byte variables are compiled to register usage, so to force a quick and effective code generation it is the right method.

User avatar
kos
Posts: 286
Joined: Thu Feb 16, 2012 4:51 am
Location: Fr

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by kos »

dongs wrote:Thanks! My compiler thanks you too.


no one use a compiler for that, unless your are using cdt and custom rules , do you know sonar ?

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by timecop »

@ all of you, declaring variables inside for loop isn't supported by all compilers, and its just shitty practice when you have 20 loops using the same variable. Just declare it on top and declare victory.

User avatar
EOSBandi
Posts: 802
Joined: Sun Jun 19, 2011 11:32 am
Location: Budapest, Hungary
Contact:

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by EOSBandi »

C99 standard allows declaring variable in the loop. So if your compiller does not allows it then it's outdated... :D

User avatar
kos
Posts: 286
Joined: Thu Feb 16, 2012 4:51 am
Location: Fr

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by kos »

dongs wrote:@ all of you, declaring variables inside for loop isn't supported by all compilers, and its just shitty practice when you have 20 loops using the same variable. Just declare it on top and declare victory.


Quoted For Posterity




:twisted:

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by timecop »

So has this been fixed yet?

User avatar
EOSBandi
Posts: 802
Joined: Sun Jun 19, 2011 11:32 am
Location: Budapest, Hungary
Contact:

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by EOSBandi »

dongs wrote:So has this been fixed yet?

No need to fix.
If you have problems with it during porting then use the -std=gnu99 with your compiller (assuming you are using gcc)

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by timecop »

I am not using gcc, and that's not an issue of code portability as much as style and having to type way too much shit.
there's no need to declare loop variable in for scope each time when its obvious its only used as a loop counter in the entire function.
it just looks dumb, forget compiler incompatibilities.

User avatar
EOSBandi
Posts: 802
Joined: Sun Jun 19, 2011 11:32 am
Location: Budapest, Hungary
Contact:

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by EOSBandi »

Earlier it was a compiller issue, now it's a stye issue.
Compiller issue : Solved
Style issue : will not engage any discussion about it.
Case closed

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by timecop »

It's a compiler issue for me, but its a style issue for you.
I think it looks ugly.
And not really necessary.
Let's see what alex says.

Liftoff
Posts: 15
Joined: Wed Jun 27, 2012 8:41 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by Liftoff »

+1 on using the C99 support also. This is a good thing.

I would trust the compiler even more, and use

for( int i=0; i<count; i++ )

I have a Cortex M3 port going, and using int for loop counters actually produces smaller code.

As to the OP's problem, maybe its time to switch to gcc or find a proper command line option for the current compiler.

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by timecop »

gcc is a terrible compiler.
doing ugly looking stuff in code "just because I can" is also stupid.
what's next, you'll start using anonymous unions and named struct member assignment?

User avatar
EOSBandi
Posts: 802
Joined: Sun Jun 19, 2011 11:32 am
Location: Budapest, Hungary
Contact:

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by EOSBandi »

DNFTT

Liftoff
Posts: 15
Joined: Wed Jun 27, 2012 8:41 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by Liftoff »

dongs wrote:gcc is a terrible compiler.


gcc is many compilers, since it gets bound to many back end code generators.
Although I have no experience with the code generator for your 8 bit chip, in general I hold a different opinion about the quality of several other code generators within the gcc family.
I have used these with great success: x86, x86_64, armv4t, armv7a, cortex m3.

doing ugly looking stuff in code "just because I can" is also stupid.


Agreed.

But have you asked anyone yet why it is being done? When I do it, it is usually for two reasons, neither of which is "just because I can".

1) scope. the variable is not needed beyond a certain region of code. This gives the compiler a chance to re-use the same machine register for a different purpose (different variable name) elsewhere in the same function.

2) human readability. it is sometimes nice to not have a cluttering of variables as you venture into a function, especially if some of those have limited use within the function. Having variables nearest to their point of use can be useful to human code comprehension.


what's next, you'll start using anonymous unions and named struct member assignment?


Is this a strategic alternative to: "please let's stick with a common subset that can be compiled by a number of compilers"?
Last edited by Liftoff on Mon Jul 02, 2012 4:45 pm, edited 1 time in total.

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by timecop »

the "code generator for your 8 bit chip" you're referring to produces ~42k hex for baseflight, which is currently on the verge of overflowing 64k flash when built with gcc with all optimizations.

Liftoff
Posts: 15
Joined: Wed Jun 27, 2012 8:41 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by Liftoff »

Hi Guys,

Please let me introduce myself. I am Dick Hollenbeck.

I am thinking about becoming a contributor to this project, if you will let me, and if it makes sense. I have little experience in copters, but tremendous experience in realtime software and software in general.

For the past 5 years, I have been a leader of software project KiCad.
http://launchpad.net/kicad
http://kicad-pcb.org

I have owned a software company for 30 years. I have been writing C code for 32 years, C++ for 24 years, Java for 17 years.
I like C++ and Java more than C.

I know very little about flying, but hope to learn quickly with your help.

It may be that I can help you all with the software. My target platform is an ARM Cortex M3, I will be doing my first flight of your software on that board in a handful of hours. I just started working with Kos on the new Java Swing GUI, although he has not seen any of my work yet. I have some new Java code ready.

Warmest regards,

Dick

User avatar
kos
Posts: 286
Joined: Thu Feb 16, 2012 4:51 am
Location: Fr

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by kos »

Welcome aboard :)


Tommie
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by Tommie »

treym wrote:https://github.com/itain/multiwii-firmware/commit/b59e11fbb8d03a6a32f53fce6bcc8b274efc2818


Hm, the premise of this patch/branch is a bit off, renaming the file to .c might break a lot of things since Arduino code is actually C++ and not plain C.

I have to admit that I personally like the declaration of the loop variable inside the loop condition; it keeps the variable scope small and prevents "semiglobal" variables that lurk around for hundreds of lines of code, but I also don't know how the optimizers handle multiple loops of that kind.

User avatar
treym
Posts: 258
Joined: Sat Jul 21, 2012 12:28 am

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by treym »

unscoped variable and cyclomatic complexity are evil

-std=c99 -pedantic

itain
Posts: 75
Joined: Tue Aug 23, 2011 10:32 pm

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by itain »

Optimized code should not care. Decent compilers can split the scope of a variable to "live range" that usually start on assignment to the variable and end in the last use before the next assignment. See more here: Live variable analysis.

The only issues are portability and readability.

acemtp
Posts: 12
Joined: Mon Jul 16, 2012 8:33 am

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by acemtp »

I really like the for(int ... ), I use it everytime I code in C++.
But I like even more the standard and the important things to know is what is the standard we/alexinparis want to use for his project and then follow the standard.

is it C:
ANSI X3.159-1989 (ANSI C, C89)
SO/CEI 9899:1990 (C90)
ISO/CEI 9899:1999 (C99)
ISO/IEC 9899:2011 (C11)

or C++:
ISO/CEI 14882:1998
ISO/CEI 14882:2003
ISO/CEI 14882:2011

When it s decided, we know what to do and what we must not do.

Tommie
Posts: 438
Joined: Sun Apr 08, 2012 9:50 am

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by Tommie »

That's simple. Since our reference if the Arduino IDE, we are clearly using C++, with whatever standard Arduino passes to gcc.

User avatar
treym
Posts: 258
Joined: Sat Jul 21, 2012 12:28 am

Re: Get rid of for (uint8_t i = blah stuff in serial.c

Post by treym »

please , lets focus ..

dongs wrote:Guys, I realize that you like to type same stuff over and over but,

in evaluateCommand() there's like 20 occurences of the thing in /subject without any reason.
Protip:

void evaluateCommand(void)
{
uint8_t i; // do it once, here

...

for (i = 0; i < .... );
}

Thanks!


https://www.google.com/search?q=scoped+ ... efactoring

http://xkcd.com/1081/

Post Reply