Archive for the ‘Coding Standards’ Category

« Older Entries
Newer Entries »

Eye, Aye I!

Friday, November 6th, 2009 Nigel Jones

Today’s post should probably be called ‘Thoughts on non-descriptive variable names’, but once in a while I have to let my creative side out!

Anyway, the motivation for today’s post, is actually Michael Barr’s latest blog posting concerning analysis of the source code for a breathalyzer. Since I do expert witness work, as well as develop products I was keen to see what the experts in this case had to say. One snippet from the expert for the plaintiffs caught my eye. In appendix B of their report, Draeger made the following statement concerning general code issues:

Non descriptive variable names – i, j, dummy and temp

This touched upon something where I seem to be at odds with the conventional wisdom. I’ll illustrate what I mean. Consider initializing an array to zero (I’ll ignore that we could use a library function for this). I would code it like this:

uint8_t buffer[BUFSIZE];
uint8_t i;
for (i = 0; i < BUFSIZE; i++)
{
 buffer[i] = 0;
}

This code would be rejected by many coding standards (and apparently would offend Draeger), as the loop variable ‘i’ is not descriptive. To be ‘correct’, I should instead code it like this

uint8_t buffer[BUFSIZE];
uint8_t buffer_index;
for (buffer_index = 0; buffer_index <BUFSIZE; buffer_index++)
{
 buffer[buffer_index] = 0;
}

So for me, the question is, does the second approach buy me anything – or indeed cost me anything? Well clearly, this is a matter of opinion. However I’d make the following observations:

  1. I think my code is clear, concise and easily understood by even the most unskilled programmer
  2. Is the variable name ‘buffer_index’ clearer – yes but only to a native English speaker. It’s my experience that there are a lot of non-native English speakers in the industry.
  3. Personally, I find the use of similar words in close proximity (buffer[buffer_index]) to be a bit harder to read, and very easy to mis-read if there are other variables around prefixed with buffer.

I’d also make the observation that many coding standards require variable names to be at least 3 characters long, and as a result I’ve seen code that looks like this:

uint8_t buffer[BUFSIZE];
uint8_t iii;
for (iii = 0; iii < BUFSIZE; iii++)
{
 buffer[iii] = 0;
}

Clearly in this case, the person is addressing the letter of the standard (if you’ll pardon the pun), but not the spirit. Where the standard requires the variable names to be meaningful, I’ve also seen this done:

uint8_t buffer[BUFSIZE];
uint8_t idx;
for (idx = 0; idx < BUFSIZE; idx++)
{
 buffer[idx] = 0;
}

This code meets the letter of the standard, and arguably the spirit. Is it really any more understandable than my original code? I don’t think so – but I’ll be interested to get your comments.

Home

Tags: ,
Posted in Coding Standards, General C issues | 7 Comments »

Whither white space?

Tuesday, October 20th, 2009 Nigel Jones

I was looking over some code I wrote a year ago in preparation for making some minor enhancements, when I noticed that in one place I had two blank lines between functions, instead of my coding standard mandated one line. I immediately and instinctively corrected it – as is my norm. However having done so, I paused to consider what I’d just done – and why.

On the one hand, this was a clear violation of my coding standard – and so it must be corrected. However, the violation wasn’t doing any harm, per se, and correcting it came at a cost – namely that someone (probably me) browsing the version control system at a later date will see that the file has been touched – and may choose to investigate what was changed – only to find out that it was a simple white space correction. (I appreciate that version control systems can be set up to ignore white space. I choose to not use that option).

Now I suspect that readers of this blog will be divided. Some will think I was quite right to eliminate the extra line, whereas others are thinking – doesn’t this guy have better things to do in life? Which brings me to my point!

Some people are completely anal retentive when it comes to white space. They are very careful on indentation, alignment of comments, use of blank lines and so on. I fall squarely into this category – as does Jean Labrosse of MicroOS II fame. Others could not care less about white space. They will arbitrarily have 6 blank lines between two functions,and then no lines between the next two functions. Their comments are usually aligned all over the place, and they rarely use space between e.g. the elements of a for loop statement. Finally, there’s the third (and largest group) who fall somewhere in between these two extremes.

Now I look at a lot of code, and  having done so, I think I can make a sweeping generalization, which I’ll call the “Nigel Jones white space principle”. Succinctly put, it states:

White space discipline is highly correlated with coding discipline.

That is, those who are careless about white space are often careless about a lot of other things. The converse seems to apply. As a result, when I look at code, literally the first thing I note is how well disciplined was the author in the use of white space. If the code is cleanly and consistently laid out, then I get a good first impression, and the chances are the code will be first rate.

Now I am unsure which is the cause and which is the effect here. In other words, does white space discipline lead to more disciplined code overall, or is it the other way around? Regardless, if your code looks like a mess, then I’d humbly suggest that you literally clean up your act – your career will thank you!

Home

Posted in Coding Standards, General C issues | 6 Comments »

Is MISRA compliance worthwhile?

Wednesday, October 7th, 2009 Nigel Jones

I had been planning on talking about MISRA C compliance in a month or two from now. However, in the comments section of my recent post about bitfields, Anand posed the following question:

However I am writing to ask your opinion about MISRA C compliance. This is the first time a client company has asked us for MISRA C compliance and I am not quite sure where to start. I started reading the guidelines from the web, however soon realised that its an enormous task and I would never get through all of them. So what I would like to know from you is how realistic is it to expect to comply with all the guidelines? Since my compiled code size is less than 2KB so should we even bother with MISRA C compliance?
Your valuable insights would be highly appreciated

I first gave my thoughts about MISRA C in an article published in 2002 in Embedded Systems Programming magazine (now Embedded Systems Design). Looking back over the article, I don’t see anything in it that I disagree with now. However, there are certainly some things that I’d add, having attempted to adhere to the MISRA guidelines in several large projects. After I have given my thoughts, I’ll try and address Anand’s questions.

I’ll start by noting that since I wrote the aforementioned article, MISRA released a second edition of their guidelines in 2004. The second edition was a major revision, and attempted to address many of the ambiguities in the 1998 version. As such, if someone is asking for MISRA compliance, they usually mean the 2004 rules; however it would behoove you to check!

Most of the MISRA rules can be checked via static analysis (i.e. by a compiler like tool) and indeed many more compilers now come with a MISRA checking option. Thus for those of you that are using such a compiler, conformance to most of the rules may be checked by simply using the correct compiler switch. For those of you that don’t have such a compiler, a great alternative is my favorite tool – PC-Lint from Gimpel – which brings me nicely to the main point I wish to make. MISRA  attempts to protect you from the darkest, nastiest corners of the C language – which is exactly what PC-Lint attempts to do as well. However, MISRA C attempts to do it by banning various constructs, whereas PC-Lint attempts to detect and inform you when you are using a construct in a potentially dangerous way. To put it another way, MISRA treats you like a child and PC-Lint treats you like an adult. As a result, I’ll take code that is ‘Lint free’ over code that is ‘MISRA compliant’ any day. I’d also add that making code Lint free is often a lot more challenging than making it MISRA compliant.

Now does this mean that I think the MISRA rules are not worthwhile? Absolutely not! Indeed the vast majority of the rules are pretty much good programming practice in a codified form. For example, rule 2.2. states: “Source code shall only use ISO9899:1990 ‘C’ style comments”. Now regardless of whether you agree with this rule or not, it’s my opinion that source code that contains C style comments and C++ style comments reflects a lack of discipline on the author’s behalf. Thus I like this rule – and I adhere to it.

Where I start to run into problems with MISRA are rules such as 20.6 “The macro offsetof in stddef.h shall not be used”. I wrote an article in 2004 for Embedded Systems Programming magazine entitled “Learn a new trick with the offsetof() macro”. The examples I give in the article are elegant and robust solutions to certain common classes of problems in embedded systems. Solving these problems without using the offsetof() macro is hard and /or tedious and / or dangerous. In short the medicine prescribed by MISRA is worse than the supposed disease.

Putting this all together, my feelings on MISRA are as follows.

  1. It’s intentions are excellent – and I wholeheartedly support them.
  2. Most of its rules are really good.
  3. There are times when I just have to say – sorry your attempts to make my code ‘safer’ are actually having the opposite effect and so as an experienced embedded systems engineer I’m choosing to ignore the rule in the interest of making a safer product. Note that I don’t do this on a whim!

Now clearly, when it comes to my third point, I can certainly be accused of hubris. However, at the end of the day my clients typically hire me for my experience / knowledge and not for my ability to follow a rule book.

As a final note, I must say that I think the MISRA committee has overall done a very fine job. Trying to come up with a set of rules for the vastly disparate embedded systems industry (I know they are only really aimed at the car industry) is essentially an impossible task.

So with that all of the above as a preamble, I think I can address Anand’s questions.

Where to Start?
Order a copy of the guidelines from MISRA. You can get an electronic copy for £10 (about $15). If you are using a C compiler that has a MISRA checking switch, then turn it on. If not buy a copy of PC-Lint. If you are not already using PC-Lint then you should be. It will be the best $389 you ever spent.

Then What?
Take a snapshot of your code base in your version control system before starting.
The first time you check your code for compliance, you will undoubtedly get an enormous number of errors. However, I think you will find that half a dozen rules are creating 90% of the problems. Thus you should be able to knock the error count down fairly quickly to something manageable. At that point you will be in to some of the tougher problems. My recommendation is not to blow off the MISRA errors, but rather to understand why MISRA thinks your constructs are unsafe. Only once you are convinced that your particular instance is indeed safe should you choose to ignore the violation.

Is it worth it for 2K of object code?
Yes – and no. With an executable image of 2K, the chances are most of your code is very tightly tied to the hardware. In my experience, the closer you get to the hardware, the harder it is to achieve MISRA compliance, simply because interaction with the hardware typically relies upon extensions to standard C – and these extensions violate MISRA rule 1.1. Thus you have no hope of making your code compliant, literally starting with the first rule. (The MISRA committee aren’t stupid, and so they have a formal method for allowing you to waive certain rules – and this is a clear example of why it’s necessary. However, it’s a tough way to start out). Does this mean that the exercise is pointless though? Probably not, as at the end of the day you’ll probably have cleaner, more portable, more easily maintained code. However, I seriously doubt that it will be compliant.

Finally, I’ll answer a question that you didn’t ask. What should I say to a client that asks for MISRA compliance? Well the first thing to determine is whether compliance is desired or required. If it’s the former, then what they are probably asking for is work that conforms to industry best practices. In which case what I normally do is explain to them that the code I deliver will be Lint free – and that this is a far higher hurdle to cross than MISRA compliance. If however MISRA compliance is a must, then you have no option other than to bite the bullet and get to work. It would probably make sense to retain a consultant for a day or two to help you get up to speed quickly.

Home

Tags: , ,
Posted in Coding Standards, General C issues | 10 Comments »

Effective C Tip #4 – Prototyping static functions

Saturday, July 4th, 2009 Nigel Jones

This is the fourth in a series of tips on writing effective C.

I have previously talked about the benefits of static functions. Today I’m addressing where to place static functions in a module. This posting is motivated by the fact that I’ve recently spent a considerable amount of time wading through code that locates its static functions at the top of the file. That is the code looks like this:

static void fna(void){...}

static void fnb(uint16_t a){...}

...

static uint16_t fnc(void){...}

void fn_public(void)
{
 uint16_t t;

 fna();
 t = fnc();
 fnb(t);
 ...
}

In this approach (which unfortunately seems to be the more common), all of the static functions are defined at the top of the module, and the public functions appear at the bottom. I’ve always strongly disliked this approach because it forces someone that is browsing the code to wade through all the minutiae of the implementation before they get to the big picture public functions. This can be very tedious in a file with a large number of static functions. The problem is compounded by the fact that it’s very difficult to search for a non static function. Yes I’m sure I could put together a regular expression search to do it – but it requires what I consider to be unnecessary work.

A far better approach is as follows. Prototype (declare) all the static functions at the top of the module. Then follow the prototypes with the public functions (thus making them very easy to locate) and then place the static functions out of the way at the end of the file. If I do this, my code example now looks like this:

static void fna(void);
static void fnb(uint16_t a);
static uint16_t fnc(void);

void fn_public(void)
{
  uint16_t t;

 fna();
 t = fnc();
 fnb(t);
 ...
}

static void fna(void){...}

static void fnb(uint16_t a){...}

...

static uint16_t fnc(void){...}

If you subscribe to the belief that we only write source code so that someone else can read it then this simple change to your coding style can have immense benefits to the person that has to maintain your code (including a future version of yourself).

Update: There’s a very interesting discussion in the comments section – I recommend taking a look.
Next Tip
Previous Tip
Home

Tags:
Posted in Coding Standards, Effective C/C++ | 8 Comments »

On the use of correct grammar in code comments

Saturday, April 11th, 2009 Nigel Jones

Back when I was in college the engineering students were fond of dismissing the liberal arts majors by doing such witty things as writing next to the toilet paper dispenser “Liberal Arts degree – please take one”. One of the better retaliatory pieces of graffito that I really liked was: “Four years ago I couldn’t spell Engineer – now I are one”. I think this appealed to me because there was more than a smidgen of truth in its sentiment. If you don’t believe me, just take a look at the comments found in most computer programs. I don’t think I’m being exactly controversial by noting that most comments :

  • Lack basic punctuation.
  • Contain numerous spelling errors.
  • Liberally use non-standard abbreviations.
  • Regularly omit verbs and / or other basic components of a sentence.

As a result, many comments are nonsensical. In fact I’ve been in situations where the comments are so badly written that it’s easier to read the code than it is the comments. Clearly this isn’t a good thing! When I question programmers about this, I typically get a shrug of the shoulders and a ‘what’s the big deal’ attitude. When pressed further, the honest ones will admit that they couldn’t be bothered to use correct grammar or spelling because it’s too much effort – and after all you can work out what they mean if you just try hard enough. They are of course correct. However taken to its logical conclusion, this is really an argument for not commenting at all – since with a bit (OK a lot) of effort it should be crystal clear what the code is doing (and why) simply by examining it.

I decided to write about this now since I recently heard from Brad Mosch concerning a pet peeve of his. He gave his permission for me to quote from his email:

I see all the time mixed occurrences of whether or not a space is used for things such as 3dB, 3 dB, 1MHz, 1 MHz, etc. I am hoping that someone in the embedded guru world propose that a space is ALWAYS used between the number and the unit of measure. That is the documented standard that was used in our technical writing at United Space Alliance and NASA. The funny thing is, even though that standard existed out there at Kennedy Space Center, not a whole lot of people knew about it because I saw the same problem in documents out there all the time. Anyway, my point is, isn’t “1 Hz” a lot more readable than “1Hz”?

I’m sure many of you may think that Brad is being overly picky. However I don’t. His real point (in the last sentence) concerns readability. If you are going to write a comment surely it should be as readable as possible? Now I consider myself a very conscientious commenter of code, and so as a matter of interest I did a quick search on my current project to see whether I was following Brad’s advice. Well I was – about 90 % of the time. I found that my style depended a bit on the units. For example, I always appended the % sign without a space, whereas mV, mA etc just about always had a space between the value and the units. You’ll be pleased to know Brad that I’ll be mending my ways!

Anyway, I’ll leave this topic for now. Next time I visit it I’ll tell you how I spell check my comments. Hopefully having read this post you’ll know why I do it.
Home

Posted in Coding Standards, General C issues | 10 Comments »

« Older Entries
Newer Entries »