Probleme mit Strings undVariablen langen Argumentlisten

Hallo,
hier ein kleiner Auszug aus meinem Programm

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

char toStringBuf[202];
char * toString(const char *format, ...)
{
	if(toStringBuf == NULL)
		exit(666);

	va_list argzeiger;
	va_start(argzeiger,format);
	vsnprintf(toStringBuf, 200, format, argzeiger);
	va_end(argzeiger);
	return toStringBuf;
}
void PrintX(const char *x)
{
	if(strcmp(x, "KLNF") == 0)
		printf("Die sind gleich\n");

	printf("Mit X:\n");

		printf(toString("bla\n%s\n", x));

	printf("Mit KLNF:\n");

		printf(toString("bla\n%s\n", "KLNF"));
}

int main()
{
	PrintX(toString("KLNF"));
	return 0;
}

Die toString funktion habe ich programmiert um z.B.

int var = 123;
toString("var = %i", var)

in "var = 123" umzuwandeln.
Erst wird "KLNF" mittels toString in "KLNF" umgewandelt und PrintX übergeben. Das das toString("KLNF") auch wirklich "KLNF" ist zeigt das strcmp.
Aber!: wenn man printf(toString("bla\n%s\n", x)); macht steht da

bla
bla

statt(wie bei printf(toString("bla\n%s\n", "KLNF"));)

bla
KLNF

Komplette Ausgabe:

Die sind gleich
Mit X:
bla
bla

Mit KLNF:
bla
KLNF

Ich vermute stark dass der Fehler in toString() liegt.
Ich hoffe ihr könnt mir helfen.

PS: Ich weiß ich könnte einfach printf("bla\nKLNF"); machen, aber bei meinem Programm wird die funktion toString manchmal benutzt und des ist irgendwie buggy.

DJohn

Hallo,

Dein toStringBuf ist eine globale Variable. Damit handelt man sich solche Fehler ein. Dein const char *x in PrintX kommt ja schon aus toString und ist ja nichts anders als ein Zeiger auf toStringBuf. Beim nächsten Aufruf von toString überschreibst Du das zuerst mit "bla\n%s\n".

Swordfish

C++kryptomatrix schrieb:

char toStringBuf[202];

	if(toStringBuf == NULL)
		exit(666);

Was soll das bei einem Array?!

DJohn schrieb:

Hallo,

Dein toStringBuf ist eine globale Variable. Damit handelt man sich solche Fehler ein. Dein const char *x in PrintX kommt ja schon aus toString und ist ja nichts anders als ein Zeiger auf toStringBuf. Beim nächsten Aufruf von toString überschreibst Du das zuerst mit "bla\n%s\n".

Macht Sinn, danke.

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

const char* toString(const char *format, ...)
{
	va_list argzeiger;
	va_start(argzeiger,format);
	char toStringBuf[201];
	if(vsnprintf(toStringBuf,  201, format, argzeiger) >= 200)
		perror("too long");
	va_end(argzeiger);
	std::string returning(toStringBuf);
	return returning.c_str();
}
void PrintX(const char *x)
{
	printf(toString("bla\n%s\n", x));
}

int main()
{
	PrintX(toString("KLNF"));
	return 0;
}

Ist das jetzt so richtig? Es funktioniert, aber ich bin mir nicht sicher ob das Objekt returning gelöscht wird, wenn die Funktion zu Ende ist und dann der zeiger nur noch "zufällig" auf "bla KLNF" zeigt?

hustbaer

C++kryptomatrix schrieb:

aber ich bin mir nicht sicher ob das Objekt returning gelöscht wird, wenn die Funktion zu Ende ist und dann der zeiger nur noch "zufällig" auf "bla KLNF" zeigt?

Genau das passiert => das Programm ist falsch. Wenn du das als DEBUG Build laufen lässt sollte der Fehler auch sofort "sichtbar" werden.

Deine toString Funktion müsste z.B. einen std::string zurückgeben damit das funktionieren kann.

DJohn

Ja, jetzt hast Du eine lokale Variable, die ungültig wird, sobald Du den Scope verläßt. Auch der Zeiger, den Du mit c_str() holst, ist nach dem return ungültig. Dass es in Deinem Test funktioniert, ist nur Zufall.
Eine einfache Lösung, das zu reparieren, hab ich nicht. Ich würde ja std:string als Rückgabewert von toString empfehlen. Der wird dann automatisch kopiert. Allerdings kann man std::string nicht als Parameter in der Ellipse ... nutzen. Falls das also jemand macht, ist Dein Code immer noch kaputt.

hustbaer

ps: Sieh dir mal boost::format an.
boost::format löst u.A. das von DJohn angesprochene Problem mit der Verwendung von std::string als vararg Parameter -- indem einfach keine vararg Parameter verwendet werden

Was überhaupt viele Vorteile hat. Dadurch kann boost::format z.B. auch erkennen wenn zu viele oder (schlimmer) zu wenige Parameter übergeben wurden.
Und es entfällt die Notwendigkeit den genauen Typ im Platzhalter anzugeben. Man kann z.B. einfach "bla\n%1%\n" schreiben - wobei %1% dann mit der Textdarstellung des 1. "Replacement-Wertes" ersetzt wird. Egal ob es ein String oder eine Zahl oder sonstwas ist.

http://www.boost.org/doc/libs/1_57_0/libs/format/doc/format.html
http://www.boost.org/doc/libs/1_57_0/libs/format/doc/format.html#examples

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

std::string toString(const char *format, ...)
{
    va_list argzeiger;
    va_start(argzeiger,format);
    char toStringBuf[201];
    if(vsnprintf(toStringBuf,  201, format, argzeiger) >= 200)
        perror("too long");
    va_end(argzeiger);
    std::string returning(toStringBuf);
    return returning;
}
void PrintX(const char *x)
{
   printf(toString("bla\n%s\n", x).data());
}

int main()
{
    PrintX(toString("KLNF").data());
    return 0;
}

So müsste es dann funktionieren.
Danke.

Schau dir noch mal hustbärs Vorschlag an...

Und meines Wissens hängt string::data() keine 0 an, bzw. sie muss es nicht, die Funktion ist daher in vielen Fällen eher nutzlos, manchmal (wie bei dir) gefährlich.
c_str() fügt die Nullterminierung an.

dwarf schrieb:

Und meines Wissens hängt string::data() keine 0 an, bzw. sie muss es nicht

Ab C++11 muss ein 0 Char angehängt sein.

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

#define cptr c_str() 
std::string toString(const char *format, ...)
{
    va_list argzeiger;
    va_start(argzeiger,format);
    char toStringBuf[201];
    if(vsnprintf(toStringBuf,  201, format, argzeiger) >= 200)
        perror("too long");
    va_end(argzeiger);
    std::string returning(toStringBuf);
    return returning;
}
void PrintX(const char *x)
{
   printf(toString("bla\n%s\n", x).cptr);
}

int main()
{
    PrintX(toString("KLNF").cptr);
    return 0;
}

So müsste des dann funktionieren.

DocShoe

Und wenn wir schon C++ programmieren binden wir auch die richtigen Header ein:

#include <cstdio>
#include <cstdlib>
#include <cstdarg>

hustbaer

C++kryptomatrix schrieb:

#define cptr c_str()

Lass sowas bleiben.
"Trying to be cute" hat nix mit ernsthafter Softwareentwicklung zu tun.