Log Funktion



  • Hallo,
    ich habe da ein kleines Problem mit einer log Funktion die ich geschrieben habe.

    Die Funktion sieht fol­gen­der­ma­ßen aus:

    void KServer::WriteLog(string FilePath, char* Message, ...){
    	char buffer[2048];
    	va_list Args;
    	va_start(Args, Message);
    	vsprintf_s(buffer, Message, Args);
    	va_end(Args);
    	ofstream Log(FilePath, ios_base::out | ios_base::app );
    		if(Log){
    			Log << buffer << endl;
    			Log.close();
    			this->ConsoleMessage("Black", buffer);
    		}else{
    			this->ConsoleMessage("Red", "Failed to open [%s]", FilePath);
    		}
    
    }
    

    so nutze ich die funktion:

    void LoginLog(int* PlayerPointer){
    	KServer Server;
    	KPlayer Player(PlayerPointer);
    	char* SystemDate = Server.GetSystemDate();
    	char* SystemDay = Server.GetSystemDay();
    	char* SystemTime = Server.GetSystemTime();
    	char* PlayerName = Player.GetName();
    	int UID = Player.GetUID();
    	int PID = Player.GetPID();
    	char* PlayerClass = Server.SwitchClass(Player.GetClass());
    	char* PlayerStatus = Server.SwitchAdmin(Player.GetAdmin());
    	char* PlayerIP = Player.GetIP();
    	Server.WriteLog("LunaLogs\\LoginLog.txt", "[Date %s (%s)] [Time %s] [AccountInfo Name:(%s) UID:(%d) PID:(%d) Class:(%s) Status:(%s) IP:(%s)]", SystemDate, SystemDay, SystemTime, PlayerName, UID, PID, PlayerClass, PlayerStatus, PlayerIP);
    }
    

    der output den ich bekomme sieht so aus:

    [Date 21:37:32 (Thursday)] [Time 21:37:32] [AccountInfo Name:(Darn002) UID:(21484) PID:(30065) Class:(Archer) Status:(Administrator) IP:(127.0.0.1)]
    

    komischerweise wird mir als Date und Time das selbe angezeigt,
    wenn ich jetzt aber "Time" weglasse wird mir das Date richtig angezeigt

    [Date 27/02/2014 (Thursday)] [AccountInfo Name:(Darn002) UID:(21484) PID:(30065) Class:(Archer) Status:(Administrator) IP:(127.0.0.1)]
    

    hier sind noch meine zeit und datum Funktionen
    funktioniert einzeln super beide zusammen aber nicht.

    char* KServer::GetSystemDate(){
    	    time_t t = time(0);
    		struct tm timeinfo;
            localtime_s(&timeinfo, &t);
    		char Date[256];
    		sprintf_s(Date, "%02i/%02i/%02i", timeinfo.tm_mday, timeinfo.tm_mon+1, timeinfo.tm_year+1900);
    		return Date;
    }
    
    char* KServer::GetSystemTime(){
    		time_t t = time(0);
    		struct tm timeinfo;
            localtime_s(&timeinfo, &t);
    		char Time[256];
    		sprintf_s(Time, "%02i:%02i:%02i", timeinfo.tm_hour, timeinfo.tm_min, timeinfo.tm_sec);
    		return Time;
    }
    

    Habe ich irgendwo einen denk Fehler?
    Ich verstehe nicht wirklich warum ich nicht Zeit und Datum gleichzeitig nutzen kann.
    wäre schön wenn jemand was dazu sagen könnte.
    grüße Christian.



  • Adressen von lokalen Daten rauslassen ist pöse.
    Einfache Heilung:

    static char Time[256];
    

    Bessere Heilung, viel besser(!) printf verlassen. Evtl variadic templates und mit User Arcoth kurz reden. Oder schlicht und einfach stringstream oder wie das hieß, ist auch supi sicher und fürs Logging eh schnell genug.



  • volkard schrieb:

    Adressen von lokalen Daten rauslassen ist pöse.
    Einfache Heilung:

    static char Time[256];
    

    Bessere Heilung, viel besser(!) printf verlassen. Evtl variadic templates und mit User Arcoth kurz reden. Oder schlicht und einfach stringstream oder wie das hieß, ist auch supi sicher und fürs Logging eh schnell genug.

    👍
    Funktioniert jetzt super danke. 🙂

    [Date 27/02/2014 (Thursday)] [Time 22:17:21] [AccountInfo Name:(Darn002) UID:(21484) PID:(30065) Class:(Archer) Status:(Administrator) IP:(127.0.0.1)]
    


  • volkard schrieb:

    Evtl variadic templates und mit User Arcoth kurz reden. Oder schlicht und einfach stringstream oder wie das hieß, ist auch supi sicher und fürs Logging eh schnell genug.

    Ich nehm gern boost::format.
    Lässt sich per Excaption-Flags so einstellen dass es nie-nicht Exception wirft wegen "malformed" Format-Strings, zu-viel/zu-wenig Parameter etc.

    Da MSVC 2005 keine Variadic-Templates kann, hab' ich die printf() -> boost::format "Übersetzer-Funktion" halt 8-fach überladen.
    Dann kann man lässig BLUBB_LOG("printf-style-format", arg1, arg2, arg3) schreiben. In brown-grass Projekten wo schon massiv mit printf-style geloggt wird, und man die Logging-Funktionen durch was "sichereres" ersetzen will, ist das fein. Muss man nicht alles umschreiben.



  • volkard empfiehlt static. Drittklassiger Moderator.



  • Ich verstehe auch ehrlich geasgt nicht wieso der lokale Puffer ein Problem sein soll. vsprintf_s ist doch eh safe.



  • public schrieb:

    volkard empfiehlt static. Drittklassiger Moderator.

    *lach*



  • hustbaer schrieb:

    Ich verstehe auch ehrlich geasgt nicht wieso der lokale Puffer ein Problem sein soll. vsprintf_s ist doch eh safe.

    Irgendwann macht er multithreading und dann knallts, wenn er zusätzlich den Log-Mutex zu klein wählt. Interessanter finde ich, daß es keinen Grund gibt, printf statt stringstreams zu nehmen, an anderen Stellen wirds Programm eh voll mit <iostream>-Kacke sein.
    Statt

    char* KServer::GetSystemDate()//bei multithreading kaputt
    

    ginge auch

    void KServer::sprintSystemDate(char (&str)[256])//mir doch egal
    

    oder

    int KServer::sprintSystemDate(char* dst,int size)//mir doch egal
    


  • Das konkrete Problem kann man mit std::string lösen.

    std::string KServer::GetSystemTime(){
            time_t t = time(0);
            struct tm timeinfo;
            localtime_s(&timeinfo, &t);
            char Time[256];
            sprintf_s(Time, "%02i:%02i:%02i", timeinfo.tm_hour, timeinfo.tm_min, timeinfo.tm_sec);
            return Time;
    }
    


  • volkard schrieb:

    hustbaer schrieb:

    Ich verstehe auch ehrlich geasgt nicht wieso der lokale Puffer ein Problem sein soll. vsprintf_s ist doch eh safe.

    Irgendwann macht er multithreading und dann knallts, wenn er zusätzlich den Log-Mutex zu klein wählt.

    Mutex zu klein?
    WTF?
    Das Problem musst du mir erklären. Ich behaupte das existiert nicht.

    Wohingegen er mit Multithreading sehr wohl ein Problem mit einem static Buffer bekommen kann.



  • tldr

    das problem ist halt, dass du einen lokalen buffer nimmst. lokale variablen sind dazu da, sie nur innerhalb eines funktions/methoden kontextes zuverwenden (execution time). du gibst aber den zeiger auf die lokale variable zurück. nachdem die funktion "beendet" ist, gibt es aber keine garantie, dass der bereich im stack von anderen funktionen nicht verändert wird, also die daten, die dort im stack stehen. jede funktion hat halt ihren stackframe, auch wenn nur die ret addr drin ist. lokale variablen interessieren nur die funktion bei ihrer ausführung in der sie deklariert sind, für alles ausserhalb ist es irrelevant.

    static würde ich auch nicht nehmen, denn bei multithreading wirst du unter garantie ein problem bekommen. wenn der selbe buffer lesend/schreibend im kontext mehrerer threads accessed wird. hier musst du threadsafe programmieren.

    eine möglichkeit ist z.b., wie volkard ja schon gezeigt hat, dass du den zeiger auf einen buffer übergibst, in den dann das datum oder whatever geschrieben wird. achte aber auch darauf, dass du nicht über die buffergrenze schreitest. kannst z.b. die size des buffers übergeben o.ä.

    by the way: du solltest generell auch mal rückgabewerte auf validität prüfen.

    außerdem:

    this->ConsoleMessage("Black", buffer);
    ...
    this->ConsoleMessage("Red", "Failed to open [%s]", FilePath);
    

    ich gehe mal davon aus, dass "Black" und "Red" die farbe des output strings angeben soll. warum diese als string angeben? ein short int reicht doch aus für solche zwecke. ka ob deine console die normale windows console oder was anderes ist, aber für sowas lassen sich bitmasks nutzen oder gleich RGBX. wüsste nicht, warum man strings dafür nehmen soll, führt ja nur zu unnötig rechenzeit/speicherplatz verschwendung.

    pps: was wird das für ein mp game? 😃



  • Achje, ich bin auch wieder mal blind. 🙄

    Sorry, ich hab nur das char buffer[2048]; in void KServer::WriteLog(string FilePath, char* Message, ...) gesehen.
    Und das macht ja wohl kein Problem, da es nirgends zurückgegeben wird.
    Jetzt verstehe ich auch endlich was ihr mit "lokale Daten rauslassen" meint. Ich dachte das wäre auf den Aufruf von vsprintf_s bezogen gewesen -- und daher war mir nicht klar was das Problem sein soll.

    Die Funktionen unten ( char* KServer::GetSystemDate() etc.) hatte ich nicht gesehen.


Log in to reply