Problems with variadic function

Question

I have the following function from some legacy code that I am maintaining.

long getMaxStart(long start, long count, const myStruct *s1, ...)
{
   long     i1, maxstart;
   myStruct *s2;
   va_list  marker;

   maxstart = start;

   /*BUGFIX: 003 */
   /*(va_start(marker, count);*/
   va_start(marker, s1);

   for (i1 = 1; i1 <= count; i1++)
   {
      s2 = va_arg(marker, myStruct *);           /* <- s2 is assigned null here */
      maxstart = MAX(maxstart, s2->firstvalid);  /* <- SEGV here */
   }

   va_end(marker);
   return (maxstart);
}

When the function is called with only one myStruct argument, it causes a SEGV. The code compiled and run without crashing on Windows XP when I compiled it using VS2005. I have now moved the code to Ubuntu Karmic and I am having problems with the stricter compiler on Linux. Is anyone able to spot what is causing the parameter not to be read correctly in the var_arg() statement?

I am compiling using gcc version 4.4.1

Edit

The statement that causes the SEGV is this one:

start = getMaxStart(start, 1, ms1);

The variables 'start' and 'ms1' have valid values when the code execution first reaches this line.

Answer 1

It's a bit suspicious that s1 is unused. Does count include s1, i.e. the total number of pointers passed in? Perhaps you want to eliminate s1 and use va_start(marker, count).

EDIT: Given the clarification in the comments, the fix is certainly

long getMaxStart(long start, long count, /* const myStruct *s1, */ ...)
{
   long     i1, maxstart;
   myStruct *s2;
   va_list  marker;

   maxstart = start;

   va_start(marker, count);

   for (i1 = 1; i1 <= count; i1++)
   {
      s2 = va_arg(marker, myStruct *);
      maxstart = MAX(maxstart, s2->firstvalid);
   }

   va_end(marker);
   return (maxstart);
}

The legacy code used s1 to clarify what ... meant, but since it interferes with the operation of varargs, you need to comment it out.

Answer 2

As written, when you pass in only one myStruct argument, s1 is bound to that argument and your va_list will be empty. Then, the first thing you do in the loop is to get the argument from that empty list, hence the NULL.

If you require at least one argument and want the compiler to type-check that for you, you'd have to do something like this:

long getMaxStart(long start, long count, const myStruct *s1, ...) {
    ...
    va_start(marker, s1);
    maxstart = s1->firstvalid; /* actually use s1 this time! */
    for (i1 = 1; i1 < count; i1++) /* different from your code */
    {
        ...
    }
    ...
}

Otherwise, you're better off just removing s1 from the function definition like Potatoswatter mentioned:

long getMaxStart(long start, long count, ...) {
    ...
    va_start(marker, count); /* not a bug */
    maxstart = -1; /* pick something resonable for your app */
    for (i1 = 0; i1 < count; i1++)
    { 
        ...
    }
    ...
}

Answer 3

va_start takes a va_list argument and the last argument just before the variadic arguments begin. You call va_arg to get the first argument after the one you specified in va_start, or the next argument after a previous call to va_arg. In your case, you have told it that s1 is the last argument before the variadic arguments with va_start(marker, s1), so when you call va_arg, it will try to get a 4th argument to your function call, but there is no 4th, so you get some strange behaviour.

You are invoking va_arg prematurely because if you supply 1 for your count argument, it will still enter the loop, even though you don't have enough arguments there. You should use:

for (i1 = 1; i1 < count; i1++)

But, if you do this, the value s1 is never used. Go with Karmastan's answer.