Coding Standards

Monday, 11 January 2010

Portable Code: Wait for the Bite

In my two previous posts (here and here), I said I don't like assertions or system calls when writing portable code. Looking back, I realize I may not have made something clear. So let me say it here.

Assertions and system calls are tools, sometimes powerful tools. They often make your life easier and sometimes are absolutely necessary to get the job done. However, they also cause problems in porting. So if you can avoid them, I think it's a great idea to avoid them. It may turn out that you use something and it does NOT come back to bite you. However, you don't know in advance what is going to bite you and what is not, so I think a programmer who has to write portable code should simply avoid these things if possible. It might make your job a little harder now, but it can save you a ton of misery later on.

Actually, there's one thing not completely correct in that last paragraph. I said, "... you don't know in advance what is going to bite you and what is not..." Sometimes you can know. Of course, sometimes the only way of knowing is because something on a previous project bit you, so for future projects you don't do that.

And I think that's why a lot of programmers don't believe me, they've never been bitten. Let me give you a couple stories.

Two companies ago, I learned that void * is not always a generic pointer. I also learned that not all compilers treat void * the same in all situations. Then at my next company, I was happy to discover that one of the coding standards said "Don't use void *." But one day I was working on a new project, let's call it project lion cub, and another programmer was using void *. I pointed out that our coding standards said not to and explained why one shouldn't use void * when writing portable code. The other programmer said I was being too paranoid and void * was a great tool.

When we tried to port lion cub to Solaris, it wouldn't. It turned out that the way Solaris and Windows treated void * was different enough to cause an incompatibility (in the way it was being used in the project). The other programmer spent a lot of time (weekend and night time) trying to reconcile the two but later on decided to just get rid of void * and use something else.

Then about two years later, this same programmer was working on another project. I noticed he was using void * in the same way, and suggested that's a bad idea. I reminded him of what happened in project lion cub. He remembered and decided not to use void *.

Here's another story. At my previous company I was working sort of as a consultant to another company who was writing crypto code. I noticed that the programmer was using enums. I pointed out that enums were not always portable. I told him why and described a situation in the past where someone who used enums found they didn't port and had a very difficult time getting rid of them while retaining backwards compatibility. This other programmer dismissed my concerns immediately.

Then he tried to port the code to another operating system. This new operating system treated enums just a little bit differently than his base OS, and the difference was just enough that his fastest course of action was to simply get rid of them.

In both those stories, someone had an opportunity to save themselves some headaches, but they didn't believe the advice. Once they were bitten, they did believe.

I think this is one thing that happens when I talk about portability. People just don't believe me until they see it with their own eyes. They have to be bitten to believe.

To be fair, there are some other reasons. First and foremost is that I'm almost certainly TOO paranoid when it comes to porting. I believe in a very safe route, sometimes safer than it needs to be. Second, many of the porting issues I have encountered 10 or 15 years ago, aren't issues anymore. And third, it is possible to use void * and enums in a portable way.

That third is an important issue. I say, "Don't use void * and enums." Someone else might say, "Go ahead and use void * and enums, but do the work necessary to make sure they are portable."

My philosophy is, "Write to a lowest common denominator, don't even tempt fate by using things that have been problematic in the past." Another philosophy might be, "Use all the tools at your disposal, just make sure you know how to use them in a portable fashion."

Either philosophy requires more work up front but has big payoffs later on. I still prefer my philosophy, after all, you can't really know what all the porting issues are until you try to port. So how can you know how to "use them in a portable fashion"? Or another argument is this: because you know you're going to have to do more work up front either way, why not choose the philosophy that's safer? Same work effort, safer porting.

What are you going to do when you run across a company that uses 16-bit chips or an old OS (maybe they've figured out a way to make a profit on outdated but extremely cheap technology) or a company that builds embedded devices or something else you can't predict? Maybe you've used the tools in a portable way, but your portable way covers Windows, Linux, Solaris, mainframe and most or all of the most common targets. But we just don't know what someone is going to come up with.

Thursday, 07 January 2010

Avoid C Runtime, C library, System Calls in Portable Code

In a previous post, I mentioned that you should avoid using system or C runtime calls in portable code. I also said that an entire blog post would be needed to explain why. Well, here's that post.

In the C programming language there are C runtime calls. These are subroutines you can call to perform various common functions. They include memory allocation and movement (malloc, memcpy, memset), string operations (strlen, atoi), and many more.

There are also C library calls. These are essentially the same thing as C runtime calls, except that they aren't "officially" C runtime calls. For example, there are math functions such as sin, exp, and so on, or time functions.

System calls are things the kernel or operating system does such as creating new processes, threading, or file I/O. Generally there are libraries continaing routines that perform these functions so you don't have to call the kernel directly, which means they can look just like C library calls (fopen, for example).

Of course, when you get right down to it, all C runtime and C library calls are system calls with a C library interface, but somehow the programming literature makes a distinction between them.

These calls are vital to almost any program. Almost every C program written will use at least one C runtime call and a very large percentage of programs will make a C library or system call. These calls will be necessary to complete the task. It will be impossible to write many programs without calling these functions. In some programs it might be possible to get around the calls, but the performance hit might be too great, or the time spent duplicating the functionality would be a waste of company money.

So why do I say avoid them if they are vital?

First of all, I say that only for portable code. Code that will run only on Windows or only on HP/UX, for example, can be written to that platform's specification. But if your code will need to compile on Windows and Solaris and OpenBSD and Palm OS and who knows what else, then you need to write portable code, and C runtime, C library, and system calls are the biggest enemy of portable code.

Secondly, I say that if you need to use them, then you should use them through a wrapper.

One of the first questions I get when I talk about this subject is, "But aren't all these calls standardized now?" The answer is yes, but there are still some variations and other issues. For an example, let's look at malloc.

The function malloc is standardized, so why can't I use that? First, there have been platforms in the past, and there might still be some that have no malloc. In the old Palm OS, to allocate memory, you called MemPtrNew (I think that's still the case, or if there is now a malloc the recommendation is to use MemPtrNew). Some embedded or small device platform might not have memory, they only have stack space. If something as basic as malloc is not portable, can you see why any other function might not be?

Another issue might be that malloc exists, but you want to use something else. For example, on Windows, there are cases where GlobalAlloc is preferred over malloc. Or maybe in your app, you want to guarantee that every allocation automatically zeroes out the memory or before every free you want to overwrite the buffer (maybe it contained passwords or other sensitive data).

Finally, the malloc function has a particular signature. It has one argument, the size, and returns the allocated buffer or NULL if the allocation fails. What if you want to allocate memory and keep track of each allocation (this makes FIPS certification easier, by the way)? Each time you allocate memory you want to pass in some sort of structure or context that does the bookkeeping. You want your memory allocation function signature to be one that allows this extra context.

This means when your code needs to allocate memory, it should call a wrapper. For example, let's say the wrapper is

int Z3Malloc (unsigned int size, Pointer ctx, Pointer *buffer);

In your program, every call to "malloc" will be a call to Z3Malloc. If you need to change the underlying malloc, you do it once in your code.

That was malloc, how about threading? Your app might want to do some synchronization. On Windows you use Windows threading and on Linux you use Pthreads. The wrapper makes even more sense. How about file I/O and getting the current time? There are some standard functions, but there is still some variability in the real world.

Another big issue is improved versions and deprecation. For example, suppose your code uses snprintf.

  someLen = snprintf (
    buf, sizeof (buf), "%s//specialLocation", bankName);


On Windows you have to use _snprintf, and then that function is deprecated, you're supposed to use _snprintf_s. If you can get away without using snprintf, great, you don't have that problem. If you must use it, put a wrapper around it so you can change it to _snprintf on Windows, and then if you want to change it to use _snprintf_s, you do it one place. If you don't change it, then your customers might see this.

warning C4996: '_snprintf': This function or variable may be unsafe.

Do you want your customers to see that message when compiling or using your code?

What about character sizes? In the US you might deal with only ASCII characters and each character is a single byte. What if you have a customer in Japan, or Russia, or even Germany, where characters might be 2 bytes long. How does your snprintf work there?

It would be better if you did not use snprintf at all. If you figure out how to do the task without using snprintf (maybe you use memcpy to complete part of the task), you will save yourself some headaches and the code might be smaller and faster. The next few paragraphs discuss that.

There are some routines that you might use that can be performed by other functions. For example, look at the following line of code.

 sprintf (header, "Title information");

That call is functionally equivalent to a memcpy. Your app will probably have a wrapper for memcpy (that is such an important and often-used function). So rather than a sprintf, use this.

  #define HEADER_1 "Title information"
  #define HEADER_2_LEN 17
  #define HEADER_2 "Some other title"
  #define HEADER_2_LEN 16

  char *specificHeader;
  unsigned int headerLen;

  specificHeader = HEADER_1;
  headerLen = HEADER_1_LEN;
  Z3Memcpy (header, (unsigned char *)specificHeader, headerLen);

You might look at it and think it made something simple into something overly complicated. But here are the advantages. (1) I don't have to worry about sprintf, no wrapper, no issue about compatibility, deprecation, safety, anything. (2) Memcpy is almost certainly faster and smaller than sprintf. (3) You have made it easier to deal with ASCII/EBCDIC issues if they come up. (4) It's easier to change or add header info.

Actually, you can get numbers 3 and 4 working for you using sprintf as well, but I thought I'd throw them in to illustrate a little bit of how to write robust code.

There are many other examples of using wrappers. Suppose you want to do any networking. What is the socket API? Or how do you download something using HTTP? The list could be very long.

If you can write your code in such a way that it avoids a system call you will make your life easier when it comes to porting. If you must use a system call, use it through a wrapper. But the fewer wrappers the better. After 15 years of porting code, I have learned that you just never know what is going to pop up at you.

Tuesday, 05 January 2010

Why I Don't Like Assertions

***Note that this is not a Luther Martin post. It's the first Steve Burnett blog post in months.***

In the C and C++ programming languages there is something called an "assertion". It can look like this.

int Subroutine (int arg1, Pointer arg2) {
  ...
  assert (arg2 != NULL);
  ...
}

The idea is that when your code is compiled in debug mode, it will assert that that arg2 is not NULL. If arg2 is NULL, the program will either break (if in the middle of a debug session) or will crash. In this way, during the development and test phase, you can more easily find bugs.

The most common use of assertions is to test incoming arguments and return values from subroutine calls, however, you can use them for anything. Also, these tests are made only when the code is compiled in debug. The assertions go away in the released version of the code.

It sounds as if assertions are such a great feature, how could anyone possibly object to them? Here are my reasons for not using them.

1. It's system or C runtime code. Here at Voltage we write portable code. Our code has to run on Windows, Linux, Solaris, mainframe, HSM, POS, and other platforms. The biggest porting problem is system or C runtime calls. These are the basic memory allocation, file manipulation, printing, networking, and other such functions. If your code is to be ported to other platforms, you should avoid making such calls (there are often ways to get around them), or if you must call them, you should call them through a wrapper (don't call malloc, call Z3Malloc, for example). Why this is the case would be the subject of a series of blog posts in itself, but based on my 15+ years of porting experience, if you can avoid making a system or C runtime call, avoid it on principle alone.

2. If a check is worth making in debug mode, it's probably worth making in release mode. Let's say your assertion is testing input arguments or possibly the return value of another subroutine. So you test to make sure something is not NULL or that an int arg is not negative (for instance). Why not test those conditions in release mode? The idea is that testing variables takes time. Take that time in debug mode, but save a few cycles and some code size in the release version. Here at Voltage, execution time and space is so dominated by elliptic curve and pairing operations, that 10, 20, or even 100,000 cycles (or 50 to 100 extra bytes of code size) spent on variable testing would be unmeasurable. It would be less than a blip on the total program. Release code is more important that debug code, so wouldn't it be more important to make the checks there? For example, if you don't want to divide by zero, you especially don't want to divide by zero in the release version, so why check for that condition using an assertion when that means only the debug version will make the check?

3. Testing will find these problems anyway. Suppose you are the developer and are running through the code in a unit test. Also, let's say you did not put in an assertion to check a particular input arg for NULL. And let's go further and say that somehow that arg was NULL. When you try to use that arg, you'll get a NULL pointer exception or crash, or the debugger will stop at this point in the code (isn't that pretty much functionally equivalent to what would have happened if you had used an assertion?). That is, even without an assertion, you'll find that NULL pointer. And if your unit test didn't find it, then the regular testing (QA) had better find it.

4. What do you do when an assertion finds a problem? You either make sure the calling subroutine passes in the correct arg or a called subroutine never returns a bad result (see number 3 above). Or you realize that the calling or called routine can't make those guarantees and so you have to replace the assertion with a "real" check (one that will make it to the release code) (see number 2 above). What does the assertion buy you that testing or a real check does not?

5. It's one more place that release code can go bad. Suppose the assertion somehow makes it into release code (low probability, but possible). Now your release code might crash. Wouldn't it be preferable for release code to handle errors gracefully (return a NULL Pointer error rather than crashing)? That brings us back to #2 above, but it's also about writing safe code. If there's code that is not supposed to be in the release version, don't put it into the source file.

Those are reasons to simply avoid assertions. Think of it this way, the assertion is a tool that can be used, but it comes with some drawbacks. Do the pluses outweigh the minuses? In my opinion they do not. It's a tool that can be replaced with other tools. Because there are alternatives and those alternatives don't come with the same problems assertions have, use the other tools.

Thursday, 29 January 2009

Security Errors in Code, Part 3

In Part 1 I said one of the main reasons the "Top 25 Most Dangerous Programming Errors" exist is lazy programming. In Part 2 I gave the following as an example.

void DisplayMsg (char *domain, char *file) {
  char *b, *p = "Processing  from "
;
  int d = strlen (domain);
  int f = strlen (file);
  int l = strlen (p);


  b = malloc (d + f + l + 1);
  memcpy (b, p, 11);
  memcpy (b + 11, file, f);
  memcpy (b + 11 + f, p + 11, 6);
  memcpy (b + 17 + f, domain, d + 1);

  DisplayString (b);
}

I pointed out that not checking whether the malloc succeeded or not was an example of lazy programming. But I said there were more examples in that code. Here they are.

1. The function name. "DisplayMsg" could mean a lot of things, but this routine really displays a specific type of message built from a domain and file name. A better name would be "DisplayMsgFromDomainFile". "But," the lazy programmer exclaims, "that's a lot to type. I can code faster if I have less to type." Right, and how much typing you do is more important than good code. Other people will have to use and maintain this code, the more descriptive you are, the easier it is.

2. Single-character variable names. In Java, longer variable names increase code size, but not in C, so there's no reason to use single-character variable names. Besides, in Java, there are obfuscators that will convert descriptive variable names to single characters to reduce code size. In fact, think of it this way, if you use single-character variable names, you are, by definition, writing obfuscated code. In the above example, the single-character variables are not that bad, but lazy here means lazy elsewhere. Single-character variable names are harder to search for and it's harder for someone to know what they mean. And look at the variable lower-case L, is that a one or an ell?

3. No free. The buffer is allocated but never freed.

4. No comments. There's no description of what this function does, nothing about the arguments (is domain allowed to be NULL?). Although this is a simple routine, it will still take someone reading this a couple minutes to figure out if this routine does what they want it to do and how to use it. Comments would be easier to follow.

Documentation is important in code. People using your code and people updating and maintaining your code will need to know what the function is supposed to do. Descriptive function and variable names are part of documentation. Comments are part as well.

I have heard programmers declare that their code is so well-written, they don't need comments. Well, if you use descriptive function and variable names, that might be true to some extent. But suppose you're writing a multi-precision Montgomery multiply routine. We can know that's what the function does, but in the code itself, you do a shift, why? There's a 32-bit value called n0, what is that?

"If anyone is reading this code, they must know Montgomery multiplication first, they shouldn't have to learn it by reading this code." True, but what part of MontMul is a particular part of the code doing? Is that shift by 2 really a divide by 4 or a shift for some other reason? If there's a bug in the code, debugging will be made so much easier if there are comments.

Personally, I think it's bad to say that if you want to know what a function does, read the source code. There really are people who say that, usually in the O---l community. The problem is that there can be so many functions it would take forever to go through a .h file, look at a semi-descriptive name and then look at the source code for that routine (which contains single character variable names and calls other subroutines, so now I'll have to understand those functions to understand the one I'm investigating).

Programmers sometimes say, "I'll put the comments in later." Maybe that has happened but I have never seen it. I have actually put comments into other people's code after the fact, but I've never seen an original programmer do that. I have only seen programmers who put the comments in while coding, or never put them in.

I once worked with a guy who couldn't get his own code to work because he couldn't remember how to use it. He wrote the code 3 months prior, but he still couldn't remember which function to call in which order with which arguments. Looking at the source code did no good. He liked to use nondescriptive function names and single character variable names. I think the only comments he ever put into code was the copyright notice. And work he did three months ago was lost. The company paid him to do something and it was a waste of money. That's why your code needs to be well-written, easy to follow, and documented.

Lazy programmers say that so long as the code gets the right answer, that's all that matters. But you can have code that gets the right answer but has security flaws and is unmaintainable.

Craft your code so that it looks good, is not just correct. Write a description of what a subroutine does (every subroutine, not just the "important" ones), and what the arguments are before you even write the code. Comment the code as you write. Everyone says they know they should and sometimes they do, but so many never follow through. If you're too lazy to write function descriptions or comments, get out of the business.

Here's an analogy. Part of the grocery store clerk's job is to handle the coupons, either scan them or manually make the appropriate deductions. Any clerk who says, "Yeah, I should, but sometimes I don't so things move faster," is not doing their job. You wouldn't say, "It's up to the discretion of the clerk to decide if the customers' coupons are to be accepted. If they don't feel like it, then oh well." No, it's part of the job.

Making sure your code is documented right from the start, making sure your code is easy to read, making sure your code is completely correct (not just "it completes the task") is part of your job. If you don't do those things, you're not doing your job. If you don't want to do it, find another job.

Voltage Data Breach Index

  • Grab the Voltage Data Breach Index

September 2010

Sun Mon Tue Wed Thu Fri Sat
      1 2 3 4
5 6 7 8 9 10 11
12 13 14 15 16 17 18
19 20 21 22 23 24 25
26 27 28 29 30