7
\$\begingroup\$

I need to write a function in C which performs a checksum using the rule "add without carry". As I understand it, the "add without carry" concept is equivalent to the bitxor operator and given that the bitxor operation is associative and commutative, I am relying on that to perform the calculation. Below is my function. Note that the input is in decimal format and the checksum is of the binary digits with final output being in decimal format.

UCHAR checksumAddWoCarry(int num, ...)
{
    va_list valist;  /* List of variable arguments */
    UINT arg1, arg2; /* The first two arguments from the va_list */
    int i;           /* Iterator */
    UINT result;     /* Storage for returned result */

    va_start(valist, num); /* Start the usage of the list */

    arg1 = va_arg(valist,UINT); /* Pull out the first two variable arguments */
    arg2 = va_arg(valist,UINT);
    result = arg1 ^ arg2;
    if (num > 2) {
        for (i = 2; i < num; i++) {
            result ^= va_arg(valist,UINT);
        }
    }

    va_end(valist);

    return (UCHAR)result;
}

An example call would be

result = checksumAddWoCarry(3, 4,5,6);

where I expect that result is 7.

I would like an answer to two related questions.

  1. Is my logic for the implementation correct? Specifically in regards to my use of the bitxor operator to perform the checksum?
  2. I want to know, aside from the logic of the implementation, if my method is a good one. Are there any gotchas I could run into? Is there a better/faster way?
\$\endgroup\$
5
\$\begingroup\$

It appears code is simply exclusive or-ing the arguments. Sounds like a "add without carry".

  1. "Is my logic for the implementation correct?"
    Appears correct except the range of acceptable values is not clearly stated.

  2. Gotcha 1. What is UINT? If UINT is narrower than int/unsigned, the arguments will be widened and va_arg(valist,UINT); is problematic. Recommend unsigned instead.

  3. Gotcha 2. arg1 = va_arg(valist,UINT); arg2 = va_arg(valist,UINT); Only makes sense if n it at least 1 or 2. Should test for that first.

  4. if (num > 2) { for (i = 2; i < num; i++) { looks dodgy.

  5. Simplify

    va_list valist;
    va_start(valist, num);
    unsigned result = 0;
    while (num-- > 0) {
      result ^= va_arg(valist, unsigned);
    }
    va_end(valist);
    return result;
    
\$\endgroup\$
  • \$\begingroup\$ Thanks for your comments. I should have mentioned that this function is designed for a specific purpose where UINT and UCHAR are defined to be unsigned int and unsigned char respectively. As for gotcha 2, I did realize that could be an issue but in my case, I'm guaranteed to have at least 2 inputs so I didn't bother checking. Thanks for the simplified version though. That really helps! \$\endgroup\$ – zephyr Jul 25 '16 at 12:55
  • \$\begingroup\$ @zephyr Consider than good code is often re-used or at least, needs to cope with changing coding goals. In that case, "guaranteed to have at least 2 inputs" may not hold. IOWs, avoid narrow code use, especially when a more general solution exists. \$\endgroup\$ – chux Jul 25 '16 at 12:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.