embedded software boot camp

Effective C Tip #5 – Use pre-masking rather than post-masking

Monday, August 17th, 2009 by Nigel Jones

This is the fifth in a series of tips on writing what I call effective C.

Today I’d like to offer a simple hint that can potentially make your buffer manipulation code a little more robust at essentially zero cost. I’d actually demonstrated the technique in this posting, but had not really emphasized its value.

Consider, for example, a receive buffer on a communications channel. The data are received a character at a time under interrupt and so the receive ISR needs to know where to place the next character. The question arises as to how best to do this? Now for performance reasons I usually make my buffer size a power of 2 such that I can use a simple mask operation. I then use an offset into the buffer to dictate where the next byte should be written. Code to do this typically looks something like this:

#define RX_BUF_SIZE (32)
#define RX_BUF_MASK  (RX_BUF_SIZE - 1)
static uint8_t Rx_Buf[UART_RX_BUF_SIZE]; /* Receive buffer */

static uint8_t RxHead = 0; /* Offset into Rx_Buf[] where next character should be written */

__interrupt void RX_interrupt(void)
{
 uint8_t rx_char;

 rx_char = HW_REG;         /* Get the received character */
 Rx_Buf[RxHead] = rx_char; /* Store the received char */
 ++RxHead;                 /* Increment offset */
 RxHead &= RX_BUF_MASK;    /* Mask the offset into the buffer */
}

In the last couple of lines, I increment the value of RxHead and then mask it, with the intention of ensuring that the next write into Rx_Buf[] will be in the requisite range. The operative word here is ‘intention’. To see what I mean, consider what would happen if RxHead gets corrupted in some way. Now if the corruption is caused by RFI or some other such phenomenon then you are probably out of luck. However, what if RxHead gets unintentionally manipulated by a bug elsewhere in your code? As written, the manipulation may cause a write to occur beyond the end of the buffer – with all the attendant chaos that would inevitably arise. You can prevent this by simply doing the masking before indexing into the array. That is the code looks like this:

__interrupt void RX_interrupt(void)
{
 uint8_t rx_char;

 rx_char = HW_REG;         /* Get the received character */
 RxHead &= RX_BUF_MASK;    /* Mask the offset into the buffer */
 Rx_Buf[RxHead] = rx_char; /* Store the received char */
 ++RxHead;                 /* Increment offset */
}

What has this bought you? Well by coding it this way you guarantee that you will not index beyond the end of the array regardless of the value of RxHead when the ISR is invoked. Furthermore the guarantee comes at zero performance cost. Of course this hasn’t solved your problem with some other piece of code stomping on RxHead. However it does make finding the problem a lot easier because your problem will now be highly localized (i.e. data are received out of order) versus the system crashes randomly. The former class of problem is considerably easier to locate than is the latter.

So is this effective ‘C’. I think so. It’s a simple technique that adds a little robustness for free. I wouldn’t mind finding a few more like it.

Next Tip
Previous Tip
Home

6 Responses to “Effective C Tip #5 – Use pre-masking rather than post-masking”

  1. steve says:

    But what if you need to check whether or not the buffer is full before writing to it?One method is to have a read index for the rx buffer (where RxHead is the write index), which is similarly incremented and wrapped when data is read out of the buffer elsewhere in the code.Any function which uses these two indices to determine if the buffer is full will need to deal with the special circumstance when the buffer wraps around, where the write index is 0, and the read index is RX_BUG_SIZE-1.However, with the suggested strategy, one (or both) indices could now also be RX_BUF_SIZE, which will make the buffer-is-full function more complex. I don't doubt that this could be done, but I wonder if it's worth the additional complexity.Put another way, this strategy is advocating that RxHead can have two values (0 and RX_BUF_SIZE) which both indicate a value of zero. If nothing else, this seems philosophically troubling!PS: Shouldn't Rx_Buf and RxHead both be declared as volatile, given that they are modified in an interrupt?

  2. Nigel Jones says:

    Hi Steve:My general response is that I was trying to be illustrative and so omitted several 'real-world' issues.Now to some specific responses.1. I have found plenty of real world scenarios where pre-masking works very well.2. If the particular case does require one to check for buffer full, then indeed pre-masking can make things more difficult. Having said that, I'll do a posting shortly concerning my experiences with run time error checking in embedded systems. I think you'll find my views slightly controversial…3. Your comment concerning volatile is mostly correct. Variables that are modified in an ISR should normally be declared volatile. There are however exceptions, which again I will address in another blog posting.

  3. vishal says:

    really nice tips for beginner,thank you very much

  4. Dave Kellogg says:

    One thing that bothers me about this approach is that it potentially leaves RxHead pointing outside of the Rx_Buf[]. This will happen when incrementing RxHead when it is already the highest allowed subscript. The problem is that it is not until the *subsequent* invocation that RxHead is brought back in range. This feels like it violates a post-condition.

    I’d rather use the first example, and add an assertion that (Rx_Head < RX_BUF_SIZE) before using it to index into the array. This also has the benefit of loudly announcing that Rx_Head has somehow been whacked, rather than silently constraining it within bounds.

  5. Milorad says:

    Wouldn’t a little tweak here be beneficial:

    rx_char = HW_REG; /* Get the received character */
    ++RxHead; /* Increment offset */
    RxHead &= RX_BUF_MASK; /* Mask the offset into the buffer */
    Rx_Buf[RxHead] = rx_char; /* Store the received char */

    of course, RxHead should start at -1.

Leave a Reply