tags:

views:

797

answers:

6

I have a function which is passed a list of ints, until one value is "-1" and calculates the minimum.

If the function gets called couple times, it is supposed to return the minimum between all calls.

So I wrote something like that:

int min_call(int num, ...)
{
    va_list argptr;
    int number;

    va_start(argptr, num);

    //static int min = va_arg(argptr, int); //////// the questioned line

    static int all_min = -1;

    int min = va_arg(argptr, int);

    if (min != -1)
    {
        while ((number = va_arg(argptr, int)) != -1)
        {
            if (number < min)
            {
                min = number;
            }

        }
    }

    if (min < all_min || all_min == -1)
    {
        all_min = min;
    }

    return all_min;
}

I want to know something about the marked line... why can't I call it - the compiler says because the expression being used to initialize the static int is not constant.

For some reason I remember that I can initialize a static variable and know that the initializing statement will be called only once (the first time) it's written in C++. If that line would be available it would have saved me couple variables.

Is there a difference between C and C++ in this matter?

+2  A: 

You can't do what you're trying to do because the static is intialized before the program starts running -- argptr doesn't exist in context yet. You could try something like this:

va_start( argptr, num );

static bool  init = 0;
static int min = -1;
if( !init )
{
   min = va_arg( argptr, int );
   init = true;
}

static int all_min = -1;
John Dibling
+2  A: 

Static doesn't just mean it only gets set the first time you call it. It means that it's set at compile time. The compiler doesn't know how to call your va_arg function at compile time. (It doesn't make any sense until you're in run-time and running the program and passing it arguments.)

(edit - In C. Someone else has noted C++ is different.)

fennec
+7  A: 

Yes, C++ allows for statics to be lazily initialized at runtime. Effectively C++ turn static initialization into this:

static int XX_first_time = 1;
if (XX_first_time)
{
    // run the initializer
    XX_first_time = 0;
}

While this is convenient, it is not thread safe. The standard does not require this to be thread safe although some compilers have done that anyway (gcc 4.x does thread safe initialization unless explicitly requested not to with -fno-threadsafe-statics).

C requires statics be to have their value configured at compile time. Yes, this is more limited but is more in line with C doing little work for you behind your back (C can be thought of as portable assembly).

R Samuel Klatchko
Would that not be "static int XX_first_time = 1;" :-) (Joke)
Martin York
@Martin - doh! Fixed (no joke).
R Samuel Klatchko
"Only recently have C++ compilers started handling this in a thread safe manner" - in fact, they aren't even obligated to do so (since the Standard doesn't say anything about thread safety in general, and explicitly says that calling the same static function again while a static initializer inside it is still being run). E.g. VC++ doesn't do thread-safe local static initialization.
Pavel Minaev
Also, the sample expansion is subtly incorrect. The Standard requires that, if a local static initializer throws an exception, that variable is treated as not initialized - i.e., on next entry to function, the initializer should be run again. So the correct expansion should be `if (XX_first_time) { /*run initializer*/; XX_first_time=0; }`.
Pavel Minaev
@Pavel - thanks for the explanation of the subtle mistake. I've updated my example.
R Samuel Klatchko
+4  A: 

C static initialization must be done with constant values, i.e. something that is known at compile-time. Because your call to va_arg() won't resolve until runtime, you can't use it as a static initializer. You could assign a known-bad value to min at initialization and then simply call va_arg() after the variable has been initialized (John Dibling's example code does exactly that).

Also, it's important to note that static initialization only happens once per scope (in this case, your function), so even if your commented-out line worked, it would only be executed once and not each time the function is invoked (as I think you were expecting).

rpj
+4  A: 

As others have said, in C, static variables must be initialized with compile-time constants. If you omit the initial value, it is taken as 0.

Now, about the logic in your function. I think the function is written much more clearly like this (I am not changing the form of the function):

#include <limits.h>

int min_call(int num, ...)
{
    va_list argptr;
    int number;
    static int min = INT_MAX;

    va_start(argptr, num);

    while ((number = va_arg(argptr, int)) != -1)
        if (number < min)
            min = number;

    va_end(argptr);
    return min;
}

If num should also be included in the calculation of the minimum, then you will need an additional check along the lines of if (num < min) min = num;.

I also have trouble understanding if (min < all_min || all_min == -1 ) line. Since all_min was initialized to -1 in the beginning and then never touched, the if condition will always be true.

Note that the initialization of min is to INT_MAX and not INT_MIN as one of the comments says, and that we must call va_end().

Alok
note always , just the first time the function is called.but i think i will use the INT_MAX ... thanks !
Idan
I don't know what you mean by "note always". Assuming you meant "not always", what is that referring to?
Alok
Ah, I see. I don't think you need `all_min` at all, but looks like you know what's happening anyway.
Alok
+2  A: 

This doesn't exactly answer your question, but the other answers have done a good job of doing so already.

However, I think you are approaching this very simple problem from the wrong direction. If you want your functions to be portable and re-usable, you should try to minimize their side effects.

For example, instead of keeping state data in your function (evil), you should be keeping that data in a variable (or struct) local to the scope that is using it. Like this:

{
    int min_so_far;

    min_so_far = min_call(NULL, 4, 7, 2, -6, 3, -12);
    min_so_far = min_call(&min_so_far, -11, 5, 3, 8, -3);

    printf("%d\n", min_so_far); //should output -12
}

The first argument to min_call would be an int*, and if it was passed in as NULL, then the function would disregard it. Otherwise, each argument would be compared against the int that was passed in.

This isn't as convenient, but it's definitely safer, and you will be able to use this function in more than one place in the application. Thinking about the reusability of your code is a good habit to get into.

Carson Myers
You should probably declare min_so_far as an int* or else pass the address of min_so_far when you make the function call. As it is, it'll probably compile but have major logic errors (and/or a segmentation fault).
Platinum Azure
you're right, sorry, my job has had me floating in PHP land for a while and I forgot how to use a pointer properly
Carson Myers
by safe do you mean thread safe ?and why is it bad keeping state in my function ?
Idan