Being a Dick with Pointers in C

I like to argue that C can be a boring language. Boring here is a good thing. This is boring code:

void some_function(int param1, void *param2) {
    if( param1 == 0 || param2 == NULL ) {
        return;
    }
    
    char *casted_value = (char*)param2;
    if( casted_value[param1] == 'A' ) {
        //do something
    }
}

It's boring because it's obvious what each piece does, even with nonsensical variable names. If param1 is greater than 0, we check a string for the letter 'A' at position param1. Boring code is code that any novice programmer can read and understand at a quick glance. A sign of a good corporate programmer is someone who can distill any problem down into boring code.

Unfortunately, C does not force you to write boring code. Take this example from code I was looking at recently:

static unsafe public bool UnCompress(byte* source, byte* target)
{
    if (*source++ != 0x10)
        return false;

    int positionUncomp = 0;
    int lenght = *(source++) + (*(source++) << 8) + (*(source++) << 16);

    while (positionUncomp < lenght)
    {
        byte isCompressed = *(source++);
        for (int i = 0; i < BlockSize; i++)
        {
            if ((isCompressed & 0x80) != 0)
            {
                int amountToCopy = 3 + (*(source) >> 4);
                int copyPosition = 1;
                copyPosition += (*(source++) & 0xF) << 8;
                copyPosition += *(source++);

                if (copyPosition > lenght)
                    return false;

                for (int u = 0; u < amountToCopy; u++)
                {
                    *(target + positionUncomp) = *((target + positionUncomp - u) - copyPosition + (u % copyPosition));
                    positionUncomp++;
                }
            }
            else
            {
                *(target + positionUncomp++) = *(source++);
            }
            if (!(positionUncomp < lenght))
                break;

            isCompressed <<= 1;
        }
    }

    return true;
}

This is technically C#, but it's written in C basically. Remove the class decorators and gcc would compile this as-is.

This is not boring code. It's working code that solves a complex problem, but it is a pain to parse. In this function, we have:

  • A loop inside another loop inside yet another loop.
  • Bit shifting
  • With pointer manipulation
  • While using said pointers to reference arrays

If you were given this code to maintain, you would not thank the author for writing boring code. To prove this, please tell me what this line does without referencing anything:

int lenght = *(source++) + (*(source++) << 8) + (*(source++) << 16);

If you answered: "Sets length = values of 1, 2, and 3 bytes of source (remembering the first line already looked at the 0'th position), with the 3 byte shifted left 8 and the 4 byte shifted left 16," then you probably write in C for a living. This single line requires that you remember that the increment operator is applied after the entire dereference. Far more readable is:

int offset = 1;

int length = source[offset] + (source[offset+1] << 8) + (source[offset+2] << 16);
offset += 3;

Or, if you must use the increment operator, at least split it up:

int offset = 1;

int length = source[offset++]; 
length += source[offset++] << 8;
length += source[offset++] << 16;

It may not be as "elegant," but I can at least immediately understand that source is an array, and we are referencing the first 3 bytes from it.

Remember the classic quote: "Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. Code for readability."