tags:

views:

75

answers:

5

I have the following code :

#include <cstdarg>
#include <iostream>

using namespace std;

class a {
};

void fun1(a& aa, ...)
{
    va_list argp;
    va_start(argp, aa);
    char *p = 0;
    while ((p = va_arg(argp, char *)) != 0) {
        cout << p << endl;
    }
    va_end(argp);
}

void fun2(char *aa, ...)
{
    va_list argp;
    va_start(argp, aa);
    char *p = 0;
    while ((p = va_arg(argp, char *)) != 0) {
        cout << p << endl;
    }
    va_end(argp);
}

int main()
{
    cout << "fun2" << endl;
    fun2("a", "1", "2", (char *)0);
    cout << "fun1" << endl;
    fun1(a(), "1", "2", (char *)0);
    getchar();
}

Everything works fine with fun2. However, fun1 will just crash.

May I know how can I prevent from crashing, at the same time able to use class reference as 1st parameter.

Currently, it prints :

fun2
1
2
fun1

then crash.

I wish

fun2
1
2
fun1
1
2
A: 

You are passing a non-const reference to a temporary. Change the fun1 prototype to:

void fun1(a const& aa, ...)

Update:

Haven't used varargs for a long time, missed that a terminating parameter wasn't being passed. See D. Shawley's answer; you must pass a terminating parameter if you're going to use that as your interface.

janm
It crash still. I am using VC++ 2008. Does it crash on your side?
Yan Cheng CHEOK
Just checked on OSX and FreeBSD; works but by coincidence. You must pass a terminating null to terminate your argument processing loop. The actual best approach is to not use varargs at all.
janm
+2  A: 

It looks to me like you're crashing in fun2.

Because you're calling va_arg too many times and screwing up the stack.

You must only call va_arg the same number of times as there are parameters.

Anon.
No. I am only crashing in fun1 (Note that my calling sequence is fun2 followed by fun1). My implementation is referring http://c-faq.com/varargs/varargs1.html
Yan Cheng CHEOK
You'll notice that `fun1` is never output. Thus, your code is not returning from `fun2`. Also, I think this highlights the dangers of copy-pasting code you don't understand.
Anon.
OK. I fix according to D.Shawley comment. But it crash still in fun1.
Yan Cheng CHEOK
+2  A: 

Both fun1 and fun2 terminate the loop when they encounter a NULL or 0 parameter. You are never passing one. Change main to:

int main()
{
    cout << "fun2" << endl;
    fun2("a", "1", "2", NULL);
    cout << "fun1" << endl;
    fun1(a(), "1", "2", NULL);
    getchar();
    return 0;
}

Note I haven't compiled this, but it should work. You may have to follow janm's advice as well.

Update: I set down and thought about this again. You have to either:

  1. Instantiate an object of type a inside of main and pass it or...
  2. Following janm's advice and change a& aa in fun2 to a const& aa

When I tried to compile the original under g++, I was greeted with the following error:

error: invalid initialization of non-const reference of type 'a&' from a temporary of type 'a'
error: in passing argument 1 of 'void fun1(a&, ...)'

Essentially, you cannot pass a temporary variable as a non-const reference. See this SO question and this Herb Sutter GotW for some of the gory details.

D.Shawley
Thanks. It no longer crashing at fun2. But it still crash at fun1.
Yan Cheng CHEOK
+4  A: 

You can't use a reference parameter as the last named parameter with va_start. The reason is because va_start takes the address of the named parameter to find the location of the rest of the arguments. However, taking the address of a reference gives the address of the variable pointed at by the reference, not the address of the parameter itself. Your options are:

1) change the variable type from a reference to a pointer (or a non-reference if you are OK with a copy of the passed in variable).

2) Add an additional required parameter so that the reference isn't the last named parameter. The additional parameter can be a useful parameter, such as one of the char* you are going to pass to your particular function, or it can be a dummy variable you just ignore.

3) Change the definition of va_start. It's not recommended, but you can do it. See http://support.microsoft.com/kb/119394 for a non-portable redefinition.

Darryl
Ick - good catch Darryl. Yet another reason to avoid varargs like the plague.
D.Shawley
ref as va_params seem to work with gcc (see my answer and codepad). Nice point about the address problem. +1
acidzombie24
OK. I gave up. There are no easy way to do so.
Yan Cheng CHEOK
A: 

I'm sorry to hear this code doesnt work. I notice if fun2(aa is a ptr instead of a ref the code works. I also notice while trying to compile on gcc (via http://codepad.org/) you pass "a" into fun2 which is a char*. codepad/gcc complained about it not being a const char*. In codepad this code works. In my copy of VS2008 it crashes and 2010b2 as well.

My recommendation is to avoid va params but i'll assume you can't so i suggest not to use ref and use pointers. Or switch to gcc but i wouldnt do that unless there are no other (reasonable) option.

#include<cstdlib>
#include <cstdio>
#include <ios>
#include <iostream>
using namespace std;
class a {
};

void fun1(a& aa, ...)
{
    //cout<< "sizeof" << sizeof(aa) << "&aa == " << &aa;
    va_list argp;
    va_start(argp, aa);
    char *p = 0;
    while ((p = va_arg(argp, char *)) != 0) {
        cout << p << endl;
    }
    va_end(argp);
}

void fun2(const char *aa, ...)
{
    va_list argp;
    va_start(argp, aa);
    char *p = 0;
    while ((p = va_arg(argp, char *)) != 0) {
        cout << p << endl;
    }
    va_end(argp);
}

int main()
{
    cout << "fun2" << endl;
    fun2("a", "1", "2", 0);
    cout << "fun1" << endl;
    a aa;
    //cout<< "sizeof" << sizeof(aa) << "&aa == " << &aa;
    fun1(aa, "1", "2", 0);
    getchar();
}
acidzombie24