Search code examples
c++qtchecksumxorqbytearray

XOR gone wrong with its output when using a QByteArray


Recently, I created a small XOR checksum which allegedly worked (or at least in my head it did). I do not know what I have done wrong as I am following the exact same logic I used, when I have did it in C#/Java.

At the moment, it outputs rubbish values such as -87, -88 when the ints are sign and 2287800, 3485000 when unsigned. Is there a particular reason for that to happen?

Input data:

QByteArray str;

    str[0] =0xAA;
    str[1] =0xBB;
    str[2] =0xCC;
    str[3] =0xDD;
    str[4] =0xFF;
    str[5] =0x00;
    char ch = checkSum(str).data()[0];
    str[6] = ch; 
    serial.write(str, 7);

checkSum described bellow:

QByteArray MainWindow::checkSum(QByteArray &b)
{
    qint16 b_len = b.length();
    printf("b_length = %d\n", b_len);

    char val = 0x00;

    for (
       int i = 0 ; i < b_len ; i ++ ){
       val ^=  b[i];
       printf("Current: %d %d\n", val, b[i]);
    }

    return b;
}

Some output:

b_length = 6
Current: -88 1939120
Current: -81 1939120
Current: -80 1939120
Current: -69 1939120
Current: -70 1939120
Current: -70 1939120

Solution

  • There are plenty of errors in your code:

    char ch = checkSum(str).data()[0];
                    // checkSum doesn't actually return any checksum.
    
    QByteArray checkSum(QByteArray &b)
       // You should be returning a char, not a QByteArray.
       // You also should be taking a const reference to QByteArray.
    {
        qint16 b_len = b.length();        // You're truncating the length here.
        printf("b_length = %d\n", b_len); // Perhaps you should use qDebug() instead, but it's OK as quint16 is passed to printf as an int.
    
        char val = 0x00;                  // OK
    
        for (int i = 0 ; i < b_len ; i ++ ) {
           val ^=  b[i];                  // OK
           printf("Current: %d %d\n", val, b[i]);
                                          // You're passing a `QCharRef` where an int is expected.
                                          // That's the pitfall of using non-typesafe
                                          // printf. It'd have worked with qDebug()!
        }                  
    
        return b;                         // You're returning the wrong thing.
    }
    

    Here's how I'd write it:

    #include <initializer_list>
    
    char checkSum(const QByteArray &data)
    {
       char sum = 0x00;
       for (char c : data) {
          sum ^= c;
          qDebug() << "Current:" << (int)sum << (int)c;
       }
       return sum;
    }
    
    // This hopefully won't be necessary in a future Qt version.
    QByteArray makeByteArray(std::initializer_list<uint8_t> in) {
       return QByteArray(reinterpret_cast<const char*>(in.begin()), in.size());
    }
    
    int main() {
       auto str = makeByteArray({0xAA, 0xBB, 0xCC, 0xDD, 0xFF, 0x00});
       str.append(checkSum(str));
       qDebug() << str.toHex();
    }
    

    Since checkSum has nothing to do with the MainWindow, it should be a free function, not a method - not even a static method.