c - 如何使这个用于检查校验和的 C 代码更短？

Question

这是一个检查card[]提供的校验和的功能。已验证。 card[]包含六对字符。前五对组成一个十六进制数，最后一对包含校验和。如果校验和等于前五对的 XOR，则校验和有效。

它工作正常，但代码很糟糕。我试图使用嵌套循环，但它不起作用。现在我从 card[] 得到双，将它们转换为数字并检查校验和是否有效。

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<2;i++)
    {
        for(a=0;a<2;a++)
        {
        character0[a]=card[i];
        }
    }
    for(int i=2;i<4;i++)
    {
        for(a=0;a<2;a++)
        {
            character1[a]=card[i];
        }
    }
    for(int i=4;i<6;i++)
    {
        for(a=0;a<2;a++)
        {
            character2[a]=card[i];
        }
    }
    for(int i=6;i<8;i++)
    {
        for(a=0;a<2;a++)
        {
            character3[a]=card[i];
        }
    }
    for(int i=8;i<10;i++)
    {
        for(a=0;a<2;a++)
        {
            character4[a]=card[i];
        }
    }
    for(int i=10;i<12;i++)
    {
        for(a=0;a<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;


}

该示例的输入是“1E00EDE5E5F3”，因此 1E^00^ED^E5^E5 应该是 F3。

这段代码有问题。我现在看到了，因为

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

运行良好，但是

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

根本不工作。

Answer 1

使用 2D 数组而不是 6 个 1D 数组(与 character0 ，character1 ，...)

对 n 使用一维数组而不是 ( n0 , n1 , ...)

将 EOS 终止添加到传递给 strtol 的字符串中

这是一个重构版本:

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

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

    for (int i = 0;  i < 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;
}

更新:

这是一个更清洁/更简单的版本:

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

    tmp[2] = 0;
    for (int col = 0;  col < 6;  ++col) {
        int lo = col << 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;
}

更新#2:

这是一个更简单的版本:

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

    tmp[2] = 0;
    nx = 0;
    for (int idx = 0;  idx < 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;
}

更新#3:

根据一些反馈...

上面的前两个示例是消除“并行标量”变量(例如) v0, v1, v2, ... vN支持数组: v[N+1] .这允许用循环替换复制的代码。 OP 的代码有两个这样的实例 character*和 n*变量，所以我将两者都转换为数组。

当开始 [作为程序员] 时，何时使用数组并不总是很明显 [尤其是对于小数字]。在上述情况下， N是6。所以，代码可以通过剪切和粘贴来构建。

但是，如果 N如果是一个更大的数字，比如 1000，那么原始代码将无法很好地扩展。而且，阵列解决方案将变得 [更] 明显。

OP 的代码试图将两个字节从缓冲区复制到不同的 char character* 形式的数组在第一个代码块中[添加一个 EOS 字符以允许 strtol去工作]。这仍然有一个错误，因为 EOS 没有空间。

OP 的第二个 block 将使用 strtol在中间 character*产生的变量 n*变量。

我的第一个示例 [在我的原始帖子中]，将单独的标量变量转换为数组。

我的第二个示例 [在我的第一次更新中]，将两个 block /循环组合成一个，以便 character* [我用 2D chars 替换了数组]，可以用单个 tmp 消除大批。

当我在更新 #2 中做示例时，我假设 OP 的算法是正确的。我没有意识到:

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

[由编译器根据优先级] 被解释为:

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

我认为它被分组为:

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

因为这对 CRC 计算有意义 [而且我认为 OP 做得正确]。

我的细化【消除 nx】有利于运行 CRC 的数组]，基于以下标识:

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

因此，如果消息正确/完整，则对所有值(包括校验和)进行异或运算将产生最终值为零。所以，通过这样做，我修复了 OP 的第二个错误，基于一些偶然性。

其他人指出:

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

可以替换为:

return (nx == 0);

我曾对此进行过辩论，但认为该示例已经与 OP 的原始示例相去甚远，离开 return 会更清楚。顺序。而且，优化器 [可能] 会为两者生成完全相同的代码。

那时，我曾讨论过想出一个 hex像其他人建议的那样解码单个十六进制字符的函数，调用它两次并消除复制到 tmp并调用 strtol ，但是，再次，我觉得我离原始代码已经够远了。

但是，为了完整起见，这是我的最终/最佳示例，泛化为允许任意缓冲区大小:

unsigned int
hex(unsigned int chr)
{

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

    do {
        if ((chr >= '0') && (chr <= '9')) {
            chr -= '0';
            break;
        }

        chr = tolower(chr);

        if ((chr >= 'a') && (chr <= '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 < len;  idx += 2) {
        cur = hex(card[idx + 0]);
        cur <<= 4;

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

        crc ^= cur;
    }

    return (crc == 0);
}

请注意，即使是这样，也可以通过一些仔细的基准测试对速度进行更多调整......

更新 #4:

消除对 hex 的调用支持 [single] 表查找可能会更快并提供一些错误检查:

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 <= 0xFF;  ++chr)
            hex[chr] = 0xFF;

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

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

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

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

        crc ^= cur;
    }

    return (crc == 0);
}