Languages/C/tips

From UIT
(Difference between revisions)
Jump to: navigation, search
(Tip : Write readable code: rewritten)
(Tip : if (result = function_call()) warning)
 
(22 intermediate revisions by 2 users not shown)
Line 1: Line 1:
 
{{TOC right}}
 
{{TOC right}}
 
= C tips, tricks and... '''traps''' =
 
= C tips, tricks and... '''traps''' =
After ten years of C programming, let me gather some piece of code I saw...
+
I've learned most things on this page the hard way... I hope you'll enjoy them.
 +
 
 +
== Tip : choose good identifiers ==
 +
''People really ought to be forced to read their code aloud over the phone - that would rapidly improve the choice of identifiers''
 +
 
 +
''' ''Al Viro'' '''
 +
 
 +
== Tip : Always compile your own code with all warnings enable ==
 +
Using <code>gcc</code> use the <code>-Wall</code> flag.
  
 
== Trap : delay loop optimized away ==
 
== Trap : delay loop optimized away ==
Modern compiler will remove stupid code which seems to do nothing.
+
Modern compilers will remove stupid code which (seems to) do nothing.
 
<source lang='C'>
 
<source lang='C'>
  
 
/**
 
/**
  * This loop will probably take 0 nanosecond, it does really nothing
+
  * Using a decent compiler, this function will be completely optimized away.
* and the compiler can throw it away.
+
 
  */
 
  */
void zero_delay(void)
+
static void zero_delay(void)
 
{
 
{
 
     int i;
 
     int i;
Line 23: Line 30:
 
  * This loop will take some time, but using the cpu for wasting time is not a
 
  * This loop will take some time, but using the cpu for wasting time is not a
 
  * so good idea.
 
  * so good idea.
 +
* What becomes this delay when the cpu frequency changes or you change the cpu ?
 
  */
 
  */
 
void dumb_delay(void)
 
void dumb_delay(void)
Line 37: Line 45:
 
  * The good way(tm)
 
  * The good way(tm)
 
  */
 
  */
void smart_delay(void)
+
void smart_delay(uint32_t delay_us)
 
{
 
{
  /* Initialize a timer for the delay you want */
+
if (I_have_an_OS)
  /* put the cpu to sleep until the timer has fired */
+
{
 +
/* Use it ! */
 +
usleep(delay_us); /* from unistd.h on unix-likes */
  
  /* In any case if your OS already provide this kind of function, use it !*/
+
}
  #include <unistd.h>
+
else
  usleep(33);
+
{
 +
/**
 +
* Initialize a timer and put the CPU to sleep until the timer fires.
 +
*/
 +
}
 
}
 
}
 
</source>
 
</source>
  
 
== Trap : compilation and memory mapped registers ==
 
== Trap : compilation and memory mapped registers ==
Don't forget to use the <code>volatile</code> keyword when accessing a register
+
Don't forget to use the <code>volatile</code> keyword when accessing a memory mapped register
  
 
== Tip : Use static functions ==
 
== Tip : Use static functions ==
 
Trust the compiler, use <code>static</code> (<code>inline</code>) function instead of <code>#define</code>,
 
Trust the compiler, use <code>static</code> (<code>inline</code>) function instead of <code>#define</code>,
The function is far more readable, and a decent compiler will generate exactly the same code.
+
The function is far more readable, will be type-checked and a decent compiler will generate exactly the same code.
  
 
<source lang='C'>
 
<source lang='C'>
<stdint.h>
+
#include <stdint.h>
  
 
/* This function */
 
/* This function */
Line 115: Line 129:
  
 
     dummy = *some_status_register;
 
     dummy = *some_status_register;
 +
 +
    // The compiler will probably issue a warning as there as 'dummy' is never read.
 
}
 
}
  
Line 131: Line 147:
 
== Trap : if (a = b) ==
 
== Trap : if (a = b) ==
 
Remember, '=' is different from '=='. Any decent compiler used with decent options will warn you.
 
Remember, '=' is different from '=='. Any decent compiler used with decent options will warn you.
 +
For instance <code>gcc</code> will tell something like <code>warning: suggest parentheses around assignment used as truth value</code>
 
<source lang='C'>
 
<source lang='C'>
 
int a = 12;
 
int a = 12;
Line 142: Line 159:
  
 
== Tip : if (result = function_call()) warning ==
 
== Tip : if (result = function_call()) warning ==
Your compiler warn you if you test the return value in the if ? add one stage of braces around and it's gone
+
Your compiler warn you if you assign the return value in the if ? add one stage of braces around and it's gone
  
 
<source lang='C'>
 
<source lang='C'>
 
+
int a;
 
/* This should do a warning */
 
/* This should do a warning */
if (a = strlen("toto)")
+
if ( a = strlen("toto") )
 
{
 
{
   
+
 
 
}
 
}
  
/* This will not warn */
+
/* This one will not warn */
if ((a = strlen("toto"))
+
if ( (a = strlen("toto")) )
 
{
 
{
  
 
}
 
}
 
 
</source>
 
</source>
  
Line 172: Line 188:
 
</source>
 
</source>
  
And the result is... '''4''', or at least sizeof(int) which depends on your compiler!
+
If you expect to be sizeof(char), you're wrong, the response is '''sizeof(int)'''
 
+
 
+
 
+
  
 
== Tip : <code>signed</code> or <code>unsigned</code> char ?!? ==
 
== Tip : <code>signed</code> or <code>unsigned</code> char ?!? ==
Line 198: Line 211:
 
{
 
{
 
         return strlen(msg);
 
         return strlen(msg);
 +
}
 +
 +
</source>
 +
 +
== Tip : Pseudo-conditional compilation ==
 +
Code using <code>#ifdef</code> tends to be unreadable. When not necessary consider using constants. The resulting code
 +
will be the same (assuming some minor optimization done by the compiler).
 +
 +
The resulting code will be much easier to read, and to maintain.
 +
 +
 +
<source lang='C'>
 +
 +
// ifdef version
 +
void write_console(int c)
 +
{
 +
#ifdef USE_OUTPUT1
 +
  write(output1, c);
 +
#else
 +
  write(output2, c);
 +
#endif
 +
}
 +
 +
// const version
 +
#ifdef USE_OUTPUT1
 +
    static const int output = 1;
 +
#else USE_OUPTPUT2
 +
    static const int output = 0;
 +
#endif
 +
 +
void write_console(int c)
 +
{
 +
  if (output)
 +
  {
 +
      write(output, c);
 +
  }
 +
  else
 +
  {
 +
      write(output, c);
 +
  }
 
}
 
}
  
Line 218: Line 271:
 
  * Why not?
 
  * Why not?
 
  */
 
  */
#define LESS_COPY_PASTE {cout << __FUNCTION__ << endl;}
+
#define LESS_COPY_PASTE {cout << __FUNCTION__ << "\t";}
  
 
/**
 
/**
Line 277: Line 330:
 
}
 
}
 
cout << endl;
 
cout << endl;
 +
 +
/**
 +
* output : "SuperA SubA ~SubA ~SuperA"
 +
*/
 +
SubA *pSA = new SubA();
 +
delete pSA;
 +
 +
cout << endl;
 +
 +
/**
 +
* output : "SuperB SubB ~SubB ~SuperB"
 +
*/
 +
SubB *pSB = new SubB();
 +
delete pSB;
 +
 +
cout << endl;
 +
 
/**
 
/**
 
* output : "SuperA SubA ~SubA ~SuperA"
 
* output : "SuperA SubA ~SubA ~SuperA"
Line 295: Line 365:
 
*
 
*
 
* output : "SuperB SubB ~SuperB"
 
* output : "SuperB SubB ~SuperB"
 +
*
 +
* !!! ~SubB is never called !!!
 
*
 
*
 
*/
 
*/
Line 307: Line 379:
  
 
</source>
 
</source>
 +
 +
== Tip : Lost in <code>#ifdef</code> ? ==
 +
Then use <code>#error</code> directive.
 +
<source lang='C'>
 +
...
 +
#ifdef TOTO && TITI && !TUTU
 +
#error This compiler directive will immediately make the compiler exit with an error.
 +
#endif
 +
...
 +
</source>
 +
 +
== Tip : return early ==
 +
Conditional nesting will hurt readability, please consider the following examples, both doing the same thing:
 +
 +
<source lang='C'>
 +
int nested(int a, int  b)
 +
{
 +
if (a > 0)
 +
{
 +
if (b > 0)
 +
{
 +
if (a > 33)
 +
{
 +
if (b > a)
 +
{
 +
return 1;
 +
}
 +
}
 +
}
 +
}
 +
 +
return 0;
 +
}
 +
 +
int return_early(int a, int  b)
 +
{
 +
if (a <= 0)
 +
{
 +
return 0;
 +
}
 +
 +
if (b <= 0)
 +
{
 +
return 0;
 +
}
 +
 +
if (a <= 33)
 +
{
 +
return 0;
 +
}
 +
 +
if (b <= a)
 +
{
 +
return 0;
 +
}
 +
 +
return 1;
 +
}
 +
</source>
 +
 
[[Category:Languages]] [[Category:C]]
 
[[Category:Languages]] [[Category:C]]

Latest revision as of 15:12, 30 January 2017

Contents

C tips, tricks and... traps

I've learned most things on this page the hard way... I hope you'll enjoy them.

Tip : choose good identifiers

People really ought to be forced to read their code aloud over the phone - that would rapidly improve the choice of identifiers

Al Viro

Tip : Always compile your own code with all warnings enable

Using gcc use the -Wall flag.

Trap : delay loop optimized away

Modern compilers will remove stupid code which (seems to) do nothing.

/**
 * Using a decent compiler, this function will be completely optimized away.
 */
static void zero_delay(void)
{
    int i;
 
    for (i = 0 ; i < 10*1000 ; i++)
    {
    }
}
 
/**
 * This loop will take some time, but using the cpu for wasting time is not a
 * so good idea.
 * What becomes this delay when the cpu frequency changes or you change the cpu ?
 */
void dumb_delay(void)
{
    int i;
 
    for (i = 0 ; i < 10*1000 ; i++)
    {
        asm("nop"); /* replace with the nop instruction from your cpu*/
    }
}
 
/**
 * The good way(tm)
 */
void smart_delay(uint32_t delay_us)
{
	if (I_have_an_OS)
	{
		/* Use it ! */
		usleep(delay_us); /* from unistd.h on unix-likes */
 
	}
	else
	{
		/**
		 * Initialize a timer and put the CPU to sleep until the timer fires.
		 */
	}
}

Trap : compilation and memory mapped registers

Don't forget to use the volatile keyword when accessing a memory mapped register

Tip : Use static functions

Trust the compiler, use static (inline) function instead of #define, The function is far more readable, will be type-checked and a decent compiler will generate exactly the same code.

#include <stdint.h>
 
/* This function */
static uint8_t increment_func(uint8_t a)
{
    return ++a;
}
 
/* Will work as fast as this define */
#define increment(a) ((a)+1)

Trick : Initializing an empty struct

<stdint.h>
<string.h>
 
struct toto_t
{
    uint8_t a;
    uint8_t b;
    uint8_t c;
}
 
void init_this_struct(struct toto_t *toto)
{
    toto_t toto;
 
    /* This is the slow and boring way */
    toto->a = 0;
    toto->b = 0;
    toto->c = 0;
 
    /* Here is a smarter way */
    memset(toto, 0x0, sizeof(*toto));
}

Tip : Write readable code

  • First, write and test code that works can be easily understood.
  • If and only if it is too slow or memory inefficient try to optimize it.

Tip : dummy read of a register

Some register peripherals, especially on embedded systems must be read to be cleared. Here is a nice way of doing it.

/* Suppose this is the address of a memory mapped status register */
volatile uint32_t *some_status_register;
 
 
/**
 * Now suppose that we want to clear it, so here is a way
 */ 
void read_and_discard_status_reg(void)
{
    uint32_t dummy;
 
    dummy = *some_status_register;
 
    // The compiler will probably issue a warning as there as 'dummy' is never read.
}
 
/**
 * And here is another way, cleaner
 */ 
void read_and_discard_status_reg(void)
{
    /* the void cast will do the read, sure ;) */
    (void)*some_status_register;
}

Trap : if (a = b)

Remember, '=' is different from '=='. Any decent compiler used with decent options will warn you. For instance gcc will tell something like warning: suggest parentheses around assignment used as truth value

int a = 12;
int b = 13;
 
if (a = b)
{
   printf("12 = 13 ?!?");
}

Tip : if (result = function_call()) warning

Your compiler warn you if you assign the return value in the if ? add one stage of braces around and it's gone

int a;
/* This should do a warning */
if ( a = strlen("toto") )
{
 
}
 
/* This one will not warn */
if ( (a = strlen("toto")) )
{
 
}

Trap : sizeof('a')

Here is a bad joke, what is sizeof('a') ?

#include <stdio.h>
 
int main(int argc, char *argv[])
{
        return printf("sizeof('a') = %ld\n", sizeof('a'));
}

If you expect to be sizeof(char), you're wrong, the response is sizeof(int)

Tip : signed or unsigned char ?!?

gcc char type is neither signed nor unsigned. In the following code, gcc will only compile the test_char function without warning. Solution : cast your unsigned char* to a char* ((char*)msg)

#include <stdint.h>
#include <string.h>
 
int test_unsigned_char(const unsigned char *msg)
{
        return strlen(msg);
}
 
int test_signed_char(const signed char *msg)
{
        return strlen(msg);
}
 
int test_char(const char *msg)
{
        return strlen(msg);
}

Tip : Pseudo-conditional compilation

Code using #ifdef tends to be unreadable. When not necessary consider using constants. The resulting code will be the same (assuming some minor optimization done by the compiler).

The resulting code will be much easier to read, and to maintain.


// ifdef version
void write_console(int c)
{
#ifdef USE_OUTPUT1
  write(output1, c);
#else
  write(output2, c);
#endif
}
 
// const version
#ifdef USE_OUTPUT1
     static const int output = 1;
#else USE_OUPTPUT2
     static const int output = 0;
#endif
 
void write_console(int c)
{
  if (output)
  {
      write(output, c);
  }
  else
  {
      write(output, c);
  }
}

Trap : C++ : virtual destructor

Here is why you shouldn't forget to declare your destructors virtual.

/**
 * \brief Why use virtual destructors in c++, a definitive answer
 *
 * \author Marc Pignat < pim at hevs dot ch >
 */
 
#include <iostream>
 
using namespace std;
 
/**
 * Why not?
 */
#define LESS_COPY_PASTE {cout << __FUNCTION__ << "\t";}
 
/**
 * Let's define 4 classes
 *
 *    SuperA    SuperB
 *      |         |
 *      v         v
 *    SubA      SubB
 *
 * All 4 classes only have a default constructor and a destructor.
 *
 * The SuperA class has it's destructor declared *virtual* and the SuperB don't.
 */
class SuperA
{
	public:
		SuperA() LESS_COPY_PASTE
		virtual ~SuperA() LESS_COPY_PASTE
};
 
class SubA : public SuperA
{
	public:
		SubA() LESS_COPY_PASTE
		~SubA() LESS_COPY_PASTE
};
 
class SuperB
{
	public:
		SuperB() LESS_COPY_PASTE
		~SuperB() LESS_COPY_PASTE
};
 
class SubB : public SuperB
{
	public:
		SubB() LESS_COPY_PASTE
		~SubB() LESS_COPY_PASTE
};
 
int main(int argc, char *argv[])
{
	{
		/**
		 *  output : "SuperA SubA ~SubA ~SuperA"
		 */
		SubA a;
	}
	cout << endl;
 
	{
		/**
		 * output : "SuperB SubB ~SubB ~SuperB"
		 */
		SubB b;
	}
	cout << endl;
 
	/**
	 * output : "SuperA SubA ~SubA ~SuperA"
	 */
	SubA *pSA = new SubA();
	delete pSA;
 
	cout << endl;
 
	/**
	 * output : "SuperB SubB ~SubB ~SuperB"
	 */
	SubB *pSB = new SubB();
	delete pSB;
 
	cout << endl;
 
	/**
	 * output : "SuperA SubA ~SubA ~SuperA"
	 */
	SuperA *pA = new SubA();
	delete pA;
 
	cout << endl;
 
	/**
	 *
	 *                           ,--.!,
	 *                        __/   -*-
	 *                      ,d08b.  '|`
	 *                      0088MM
	 *                      `9MMP'
	 *                   hjm
	 *
	 * output : "SuperB SubB ~SuperB"
	 *
	 * !!! ~SubB is never called !!!
	 *
	 */
	SuperB *pB = new SubB();
	delete pB;
}
 
/**
 * Explaining why this language is so overly complicated is an exercise left
 * to the reader.
 */

Tip : Lost in #ifdef ?

Then use #error directive.

...
#ifdef TOTO && TITI && !TUTU
#error This compiler directive will immediately make the compiler exit with an error.
#endif
...

Tip : return early

Conditional nesting will hurt readability, please consider the following examples, both doing the same thing:

int nested(int a, int  b)
{
	if (a > 0)
	{
		if (b > 0)
		{
			if (a > 33)
			{
				if (b > a)
				{
					return 1;
				}
			}
		}
	}
 
	return 0;
}
 
int return_early(int a, int  b)
{
	if (a <= 0)
	{
		return 0;
	}
 
	if (b <= 0)
	{
		return 0;
	}
 
	if (a <= 33)
	{
		return 0;
	}
 
	if (b <= a)
	{
		return 0;
	}
 
	return 1;
}
Personal tools
Namespaces
Variants
Actions
Navigation
Browse
Toolbox