How can I make this C code for checking checksum shorter? This is a function for checking if the checksum provided by card[] is valid. card[] contains six pairs of characters. The first five pairs make up a hexadecimal number, and the last pair contains the checksum. The checksum is valid if it is equal to XOR of the first five pairs.

It is working fine, but the code is awful. I was trying to use a nested loop, but it wasn't working. Now I'm getting pairs from card[], converting them into numbers and checking whether the checksum is valid.

bool checksum(char card[])
{
int a=0;
char character0[2];
char character1[2];
char character2[2];
char character3[2];
char character4[2];
char character5[2];
long n0,n1,n2,n3,n4,n5;
char card_number;

    for(int i=0;i&lt;2;i++)
    {
        for(a=0;a&lt;2;a++)
        {
        character0[a]=card[i];
        }
    }
    for(int i=2;i&lt;4;i++)
    {
        for(a=0;a&lt;2;a++)
        {
            character1[a]=card[i];
        }
    }
    for(int i=4;i&lt;6;i++)
    {
        for(a=0;a&lt;2;a++)
        {
            character2[a]=card[i];
        }
    }
    for(int i=6;i&lt;8;i++)
    {
        for(a=0;a&lt;2;a++)
        {
            character3[a]=card[i];
        }
    }
    for(int i=8;i&lt;10;i++)
    {
        for(a=0;a&lt;2;a++)
        {
            character4[a]=card[i];
        }
    }
    for(int i=10;i&lt;12;i++)
    {
        for(a=0;a&lt;2;a++)
        {
            character5[a]=card[i];
        }
    }
    n0 = strtol(character0, NULL, 16);
    n1 = strtol(character1, NULL, 16);
    n2 = strtol(character2, NULL, 16);
    n3 = strtol(character3, NULL, 16);
    n4 = strtol(character4, NULL, 16);
    n5 = strtol(character5, NULL, 16);


if(n0^n1^n2^n3^n4==n5)     return true;
else return false;


}


The input for the example is "1E00EDE5E5F3", so 1E^00^ED^E5^E5 should be F3.

And something is wrong with this code. I see it now, because

if(n0^n1^n2^n3^n4==n5)     return true;
else return false;


is working good, but

if((n0^n1^n2^n3^n4)==n5))     return true;
else return false;


is not working at all.
Use a 2D array instead of 6 1D arrays (vs. character0, character1, ...)

Use a 1D array for n instead of (n0, n1, ...)

Add EOS termination to the string that gets passed to strtol

Here's a refactored version:

bool
checksum(char card[])
{
    char chars[6][3];
    long nx[6];

    for (int col = 0;  col &lt; 6;  ++col) {
        int lo = col &lt;&lt; 1;
        int hi = lo + 2;
        for (int i = lo;  i &lt; hi;  ++i)
            chars[col][i - lo] = card[i];
    }

    for (int i = 0;  i &lt; 6;  ++i) {
        char *ptr = chars[i];
        ptr[2] = 0;
        nx[i] = strtol(ptr,NULL,16);
    }

    if (nx[0] ^ nx[1] ^ nx[2] ^ nx[3] ^ nx[4] == nx[5])
        return true;
    else
        return false;
}




UPDATE:

Here's a cleaner/simpler version:

bool
checksum(char card[])
{
    char tmp[3];
    long nx[6];

    tmp[2] = 0;
    for (int col = 0;  col &lt; 6;  ++col) {
        int lo = col &lt;&lt; 1;

        tmp[0] = card[lo + 0];
        tmp[1] = card[lo + 1];

        nx[col] = strtol(tmp,NULL,16);
    }

    if (nx[0] ^ nx[1] ^ nx[2] ^ nx[3] ^ nx[4] == nx[5])
        return true;
    else
        return false;
}




UPDATE #2:

Here's an even simpler version:

bool
checksum(char card[])
{
    char tmp[3];
    long nx;

    tmp[2] = 0;
    nx = 0;
    for (int idx = 0;  idx &lt; 12;  idx += 2) {
        tmp[0] = card[idx + 0];
        tmp[1] = card[idx + 1];

        nx ^= strtol(tmp,NULL,16);
    }

    if (nx == 0)
        return true;
    else
        return false;
}




UPDATE #3:

Based on some feedback ...

The first two examples above, were to eliminate "parallel scalar" variables (e.g.) v0, v1, v2, ... vN in favor of an array: v[N+1]. This allowed replicated code to be replaced with loops. OP's code had two such instances for character* and n* variables, so I converted both to arrays.

When starting out [as a programmer], when to use an array isn't always obvious [particularly for small numbers]. In the above case, N was 6. So, the code could be built up by cut-and-paste.

If, however, N had been a much larger number, say, 1000, the original code would then not scale well. And, the array solution would have become [more] obvious.

OP's code was trying to copy two bytes from the buffer into different char arrays of the form character* in the first code block [to add an EOS char to allow strtol to work]. This still had a bug because there was no space for the EOS.

OP's second block would use strtol on the intermediate character* variables to produce n* variables.

My first example [in my original post], did the conversion from separate scalar variables to arrays.

My second example [in my first update], combined both blocks/loops into one, so that character* [which I had replaced with the 2D chars array], could be eliminated with a single tmp array.

When I did my example in update #2, I assumed that OP's algorithm was correct. I didn't realize that:

if (nx[0] ^ nx[1] ^ nx[2] ^ nx[3] ^ nx[4] == nx[5])


was being interpreted [by the compiler, based on precedence] as:

if (nx[0] ^ nx[1] ^ nx[2] ^ nx[3] ^ (nx[4] == nx[5]))


I assumed it was grouped as:

if ((nx[0] ^ nx[1] ^ nx[2] ^ nx[3] ^ nx[4]) == nx[5])


Because that's what made sense for the CRC calculations [and I assumed OP had done it correctly].

My refinement [to eliminate the nx array in favor of a running CRC], was based on the following identity:

(x == y) === ((x ^ y) == 0)


So, XORing all values (including the checksum), if the message was correct/intact, would produce a final value of zero. So, by doing this, I fixed OP's second bug, based on some serendipity.

Others have pointed out that:

if (nx == 0)
    return true;
else
    return false;


Can be replaced with:

return (nx == 0);


I had debated doing that, but decided that the example was already far afield from OP's original and that it would be clearer to leave the return sequence as it was. And, the optimizer would [probably] produce the same exact code for both.

At that point, I had debated coming up with a hex function that decoded a single hex char as others have suggested, calling it twice and eliminating the copy to tmp and call to strtol, but, again, felt I was getting far enough away from the original code.

But, just for the sake of completeness, here is my final/best example, generalized to allow an arbitrary buffer size:

unsigned int
hex(unsigned int chr)
{

    // NOTE: hopefully, this function gets inlined ...

    do {
        if ((chr &gt;= '0') &amp;&amp; (chr &lt;= '9')) {
            chr -= '0';
            break;
        }

        chr = tolower(chr);

        if ((chr &gt;= 'a') &amp;&amp; (chr &lt;= 'f')) {
            chr -= 'a';
            chr += 10;
            break;
        }

        // should blow up here (but there was no error checking in original)
        chr = 0;
    } while (0);

    return chr;
}

bool
checksum(const char *card,size_t len)
{
    unsigned int cur;
    unsigned int crc = 0;

    for (size_t idx = 0;  idx &lt; len;  idx += 2) {
        cur = hex(card[idx + 0]);
        cur &lt;&lt;= 4;

        cur |= hex(card[idx + 1]);

        crc ^= cur;
    }

    return (crc == 0);
}


Note that even this could be tweaked a bit more for speed with some careful benchmarking ...



UPDATE #4:

Eliminating the call to hex in favor of a [single] table lookup may be faster and provide some error checking:

int
checksum(const char *card,size_t len)
{
    static unsigned char hex[256] = { ['0'] = 0xFF };
    unsigned int chr;
    unsigned int cur;
    unsigned int crc = 0;

    // one time init of translation table
    if (hex['0'] == 0xFF) {
        for (chr = 0x00;  chr &lt;= 0xFF;  ++chr)
            hex[chr] = 0xFF;

        for (chr = 0;  chr &lt;= 9;  ++chr)
            hex[chr + '0'] = chr;

        for (chr = 0x00;  chr &lt;= 0x05;  ++chr) {
            hex[chr + 'a'] = chr + 0x0A;
            hex[chr + 'A'] = chr + 0x0A;
        }
    }

    for (size_t idx = 0;  idx &lt; len;  idx += 2) {
        chr = hex[card[idx + 0]];
#ifdef ABORT_ON_ERROR
        if (chr == 0xFF)
            return -1;
#endif
        cur = chr;
        cur &lt;&lt;= 4;

        chr = hex[card[idx + 1]];
#ifdef ABORT_ON_ERROR
        if (chr == 0xFF)
            return -1;
#endif
        cur |= chr;

        crc ^= cur;
    }

    return (crc == 0);
}