6

In the following C program i get the warning:

warning #2030: '=' used in a conditional expression.

What exactly is the problem and how do i avoid this? What is the correct way to iterate through the variable arguments?

#include <stdio.h>
#include <stdarg.h>

int Sum(int a, int b, ...)
{
    int arg;
    int Sum = a + b;

    va_list ap;
    va_start(ap, b);

    while(arg = va_arg(ap, int))
    {
        Sum += arg;
    }
    va_end(ap);

    return Sum;
}

int main(int argc, char *argv[])
{
    printf("%d\n", Sum(1, 2, 4, 8));

    return 0;
}
Gary Willoughby
  • 50,926
  • 41
  • 133
  • 199
  • your example code is broken: the loop terminates if `arg == 0`, but you never supply a `0` argument to `Sum()`; if all optional arguments have the same type, it's better to pass an array instead of using varargs and do some macro magic to make it look nice: http://stackoverflow.com/questions/1375474/variable-arity-in-c/1375636#1375636 – Christoph Jan 02 '10 at 14:52
  • Good point! but it doesn't effect the question. Your macro is nice though and an obvious better choice! – Gary Willoughby Jan 02 '10 at 15:27

5 Answers5

5

What you're doing is idiomatic, if slightly ugly C.

In order to convince the compiler you know what you're doing, though, you could wrap the assignment into an additional set of parentheses:

while((arg = va_arg(ap, int)))

That should take care of the warning.

Update:

adding parenthesis around the assignment doesn't seem to supress the warning in the C99 compiler im using (PellesC). – Gary Willoughby

What, it didn't? Then you need to make the test a little more explicit:

while((arg = va_arg(ap, int)) != 0)

should do the trick. It could also be argued to be slightly more readable.

You're going to ask me what I mean by "slightly ugly."

From working with other languages, I'm used to having a clear separation between testing and modifying. You're doing a test in that while of a value, but at the same time creating a side effect (namely reading in the next argument). As I said, this is considered pretty normal, yea "idiomatic" in C because a lot of C programmers do this; I think there are even examples of similar code in K&R.

By personal preference, I'd probably rewrite this as:

while (1) {
  arg = va_arg(ap, int);
  if (!arg) break;
  ...
}

This clearly separates the assignment from the test, and lets the loop stand alone as a (potentially) infinite loop. Many people would consider my code more ugly; as I said, it's a matter of personal preference.

Carl Smotricz
  • 66,391
  • 18
  • 125
  • 167
  • You are right, i've used a sneaky shortcut in the while test. I'm currently reading K&R and the entire book is full of clever little shortcuts like this. After reading the book i'm getting kinda used to these now and actually quite like it. But, and its a big but, it does make the code slightly harder to read. I understand why people use both and your code does indeed make things more clear. – Gary Willoughby Jan 02 '10 at 14:43
  • P.S. adding parenthesis around the assignment doesn't seem to supress the warning in the C99 compiler im using (PellesC). – Gary Willoughby Jan 02 '10 at 15:15
  • Tricks are good for textbook. I would prefer to write code which is easily understandable by others. Even if I look bit defensive coder.. – Jack Jan 02 '10 at 15:18
  • @Gary Willoughby: Sorry about the failing tip. I've updated my answer with what I hope will be more successful. – Carl Smotricz Jan 02 '10 at 15:21
  • 1
    one could use a `for`-loop as well: `for(arg = va_arg(ap, int); arg; arg = va_arg(ap, int)) {...}`; this will avoid the warning, but is ugly because of code duplication – Christoph Jan 02 '10 at 15:43
  • @Carl Smotricz: That did it. ;) Thanks for your post. – Gary Willoughby Jan 02 '10 at 15:43
1

The warning is about:

while(arg = va_arg(ap, int))

and is saying "are you sure you didn't mean:"

while(arg == va_arg(ap, int))

In this case, you didn't so you can supress it by saying:

while( (arg = va_arg(ap, int)) )

However, your code still won't work. There is no way to use variadic functions without supplying somehow the number of parameters represented by the elipsis. For example, printf does this using the % sign:

printf( "%s %d %p", x, y, z );

means there are three parameters represented by the elpisis.

In your case, you can probably use zero as a terminating value in the list of integers - that's what your loop implies.

Alok Singhal
  • 93,253
  • 21
  • 125
  • 158
0

change while(arg = va_arg(ap, int)) to while((arg = va_arg(ap, int))).

The warning is caused because you are assigning and checking the value of the assignment in one statement.

James
  • 24,676
  • 13
  • 84
  • 130
0

Yet another way to write the function:

int Sum(int a, ...)
{
    int sum = 0;
    int current = a;

    va_list args;
    va_start(args, a);

    for(; current; current = va_arg(args, int))
        sum += current;

    va_end(args);

    return sum;
}

It get's rid of the (useless) second named parameter and moves the increment of the argument pointer out of the loop condition.

Christoph
  • 164,997
  • 36
  • 182
  • 240
0

Example Code

#include <stdio.h>
#include <stdarg.h>

int add(int num, ...) {
    va_list vaList;
    int sum = 0;
    va_start(vaList, num);
    for (int i = 0; i < num; ++i) {
        sum += va_arg(vaList, int);
    }
    va_end(vaList);
    return sum;
}

int main() {
    printf("%d\n", add(1, 1));
    printf("%d\n", add(2, 1, 2));
    printf("%d\n", add(3, 1, 2, 3));
    printf("%d\n", add(4, 1, 2, 3, 4));
    printf("%d\n", add(5, 1, 2, 3, 4, 5));
    return 0;
}

Output: 1

3

6

10

15

duyuanchao
  • 3,863
  • 1
  • 25
  • 16