MySQL-Ergebnis an Thread senden - Referenzsemantik?

[blackdrake]

Hallo.

Ich führe eine MySQL-Abfrage aus und leite das Ergebnis in einen neuen Struct, der widerum an einen Detached-Thread übergeben wird.

Es werden also die nächsten MySQL-Zeilen durchgegangen, obwohl der Thread noch aktiv ist.

Meine Fragen lauten nun:
- Gilt bei der Zuweisung k->url = row[1] die Wertesemantik oder die Referenzsemantik? (ich befürchte Letzteres weil es beide Char-Pointer sind)
- Was passiert, wenn es die Referenzsemantik ist? Verhindert ein Referenzzähler, dass der Speicherbereich, auf den k->url zeigt, freigegeben oder verändert wird, wenn row sich ändert oder die Funktion mysql_abfrage() beendet wird?
- Oder muss ich row[1] vorher erst klonen (wie?), damit ich den Wert sicher an den Thread geben kann? (Problem 1)
- Kann ich an den übergebenen url-Wert noch etwas anhängen oder gehen dadurch im Speicher kaputt? (Problem 2)
- Kann ich free(k) verwenden und danach noch sicher auf "url" zugreifen (Problem 3)
- Ist der Code so korrekt und sauber? (Ich leide nämlich im Moment massiv unter Speicherzugriffsverletzungen...)

static void async_function_call( void* (*start_routine)(void*), void* arg ) {
        pthread_t mythread;
        pthread_attr_t tattr;

        pthread_attr_init(&tattr);
        pthread_attr_setdetachstate(&tattr, PTHREAD_CREATE_DETACHED);

        pthread_create(&mythread, &tattr, start_routine, arg);

        pthread_attr_destroy(&tattr);
}

struct myserver {
        uint    id;
        char*   url;
};

static void* meine_funktion(void* data) {
        // Lese die übergebene Struktur aus
        struct myserver *k = (struct myserver*)data;

        int id = k->id;
        char* url = k->url;

        free(k); // PROBLEM 3: Soll ich k erst freigeben, wenn ich die Variable (Alias?) "url" nicht mehr brauche?

        strcat(url, "index.html"); // PROBLEM 2: Zerhaut mir das was im Speicher, da url scheinbar ein Alias von dem ehemaligen row[1] ist?

        printf(url);

        return 0;
}

static void* mysql_abfrage(void* data) {
        MYSQL_RES* result = NULL; // MySQL-Result
        MYSQL_ROW row; // MySQL-Zeile

        result = _mysql_query("SELECT `id`, `address` FROM `webseiten` WHERE `active` = '1'");

        if (result != NULL) {
                if (mysql_num_rows(result) > 0) {
                        row = mysql_fetch_row(result);
                        while (row) {
                                struct myserver *k = malloc(sizeof *k);
                                k->id = atoi(row[0]);
                                k->url = row[1]; // PROBLEM 1: Muss der char* erst geklont werden, damit ich ihn sicher an den Thread geben kann?
                                async_function_call(meine_funktion, k);

                                row = mysql_fetch_row(result); // Was passiert nun mit dem row[1]? Zeigt der Speicherbereich k->url nun auf eine als frei gekennzeichnete Heap-Stelle?
                        }
                }
        }

        return 0; // Was passiert mit den ehemaligen row[1] Zeigern? Zeigen die k->url Speicherbereiche nun auf eine als frei gekennzeichnete Heap-Stelle?
}

Bitte um Hilfe.

Gruß
blackdrake

[~fricky]

blackdrake schrieb:

- Gilt bei der Zuweisung k->url = row[1] die Wertesemantik oder die Referenzsemantik?

referenzsemantik. aus row[l] wird zwar ein ein zeiger gemacht, der als neuer wert k->url zugewiesen wird, aber die daten, auf die gezeigt wird, werden nicht kopiert.

blackdrake schrieb:

- Was passiert, wenn es die Referenzsemantik ist? Verhindert ein Referenzzähler, dass der Speicherbereich, auf den von k->url gezeigt wird, freigegeben oder verändert wird?

nee, so 'nen zähler müsstest du dir selber bauen, wenn du ihn brauchst.

blackdrake schrieb:

- Oder muss ich row[1] vorher erst klonen (wie?), damit ich den Wert sicher an den Thread geben kann? (Problem 1)

kommt drauf an. wenn sicher ist, dass row[1] nicht verändert wird, während ein thread damit arbeitet, dann nicht. ansonsten könnteste dir 'ne kopie ziehen, z.b. mit 'strcpy'.

blackdrake schrieb:

- Kann ich an den übergebenen url-Wert noch etwas anhängen oder gehen dadurch im Speicher kaputt? (Problem 2)

wenn noch genug platz ist, dann ja. sonst nicht.

blackdrake schrieb:

- Kann ich free(k) verwenden und danach noch sicher auf "url" zugreifen (Problem

jein, du darfst nicht mehr über k->url darauf zugreifen, wenn k ge-free'd wurde. aber über row[l] geht's immer noch.

blackdrake schrieb:

- Ist der Code so korrekt und sauber? (Ich leide nämlich im Moment massiv unter Speicherzugriffsverletzungen...)

^^die frage hast du dir doch schon selbst beantwortet

[blackdrake]

~fricky schrieb:

jein, du darfst nicht mehr über k->url darauf zugreifen, wenn k ge-free'd wurde. aber über row[l] geht's immer noch.

url ist ja ein Alias von k->url. Das heißt also, ich darf free(k) erst ausführen, wenn ich auf url nicht mehr zugreifen möchte, korrekt?

~fricky schrieb:

kommt drauf an. wenn sicher ist, dass row[1] nicht verändert wird, während ein thread damit arbeitet, dann nicht. ansonsten könnteste dir 'ne kopie ziehen, z.b. mit 'strcpy'.

Ok, ich möchte schon gerne mit Arbeiten, weil ich ja "index.html" anhängen will, deswegen wäre ein Kopie sinnvoll.

Das andere Problem ist ja, dass beim verlassen der Funktion row[1] und damit k->url auf eine als frei gekennzeichnete Stelle zeigt, wenn ich mich nicht irre.

Ich habe mal folgende Funktion gebastelt:

static char* strclone(char* str) {
        #ifndef __cplusplus
        char *newstr = malloc(strlen(str)+1);
        #else
        char *newstr = (char*)malloc(strlen(str)+1);
        #endif

        memcpy(newstr, str, strlen(str)+1);

        return newstr;
}

k->url = strclone(row[1]);

Bei meine_funktion müsste k->url nun eindeutig sein. Es kommt aber bei strcat(url, "index.html") trotzdem wieder zum Speicherfehler.

--

// Apdx: Aktualisierter Code:

static char* strclone(char* str) {
        #ifndef __cplusplus
        char *newstr = malloc(strlen(str)+1);
        #else
        char *newstr = (char*)malloc(strlen(str)+1);
        #endif

        memcpy(newstr, str, strlen(str)+1);

        return newstr;
}

static void async_function_call( void* (*start_routine)(void*), void* arg ) { 
        pthread_t mythread; 
        pthread_attr_t tattr; 

        pthread_attr_init(&tattr); 
        pthread_attr_setdetachstate(&tattr, PTHREAD_CREATE_DETACHED); 

        pthread_create(&mythread, &tattr, start_routine, arg); 

        pthread_attr_destroy(&tattr); 
} 

struct myserver { 
        uint    id; 
        char*   url; 
}; 

static void* meine_funktion(void* data) { 
        // Lese die übergebene Struktur aus 
        struct myserver *k = (struct myserver*)data; 

        int id = k->id; 
        char* url = k->url; 

        strcat(url, "index.html"); // Hier kracht es scheinbar

        printf(url); 

        free(url); // Auf ein malloc von strclone() muss auch ein free folgen
        free(k);

        return 0; 
} 

static void* mysql_abfrage(void* data) { 
        MYSQL_RES* result = NULL; // MySQL-Result 
        MYSQL_ROW row; // MySQL-Zeile 

        result = _mysql_query("SELECT `id`, `address` FROM `webseiten` WHERE `active` = '1'"); 

        if (result != NULL) { 
                if (mysql_num_rows(result) > 0) { 
                        row = mysql_fetch_row(result); 
                        while (row) { 
                                struct myserver *k = malloc(sizeof *k); 
                                k->id = atoi(row[0]); 
                                k->url = strclone(row[1]); // String für den Thread einzigartig machen, damit der Pointer beim Verlassen der Funktion nicht ungültig wird
                                async_function_call(meine_funktion, k); 

                                row = mysql_fetch_row(result); 
                        } 
                } 
        } 

        return 0;
}

[~fricky]

blackdrake schrieb:

~fricky schrieb:

jein, du darfst nicht mehr über k->url darauf zugreifen, wenn k ge-free'd wurde. aber über row[l] geht's immer noch.

url ist ja ein Alias von k->url. Das heißt also, ich darf free(k) erst ausführen, wenn ich auf url nicht mehr zugreifen möchte, korrekt?

nee, du darfst nur nicht mehr über k->irgendwas darauf zugreifen. beispiel:

char *a = "hallo", **b, *c;

b = malloc (sizeof(char**));
*b = a;
c = *b;

free (b);  // b ist jetzt weg, aber c bleibt davon unberührt 

puts(c);   // ok
puts(*b);  // lieber nicht

[namespace invader]

blackdrake schrieb:

Bei meine_funktion müsste k->url nun eindeutig sein. Es kommt aber bei strcat(url, "index.html") trotzdem wieder zum Speicherfehler.

Das liegt daran, dass du für url nur genausoviel Speicher gemalloct hast, wie der ursprüngliche String benötigt hat (ermittelt mit strlen). Wenn du jetzt noch mit strcat etwas ranhängen willst, reicht es natürlich nicht mehr.

Du musst also so etwas machen wie

char *suffix = "index.html";
char buffer[strlen(url) + strlen(suffix) + 1];
strcpy(buffer, url);
strcat(buffer, suffix);

printf(buffer);

[blackdrake]

Hallo.

Danke für eure Antworten.

Das Beispiel habe ich leider nicht verstanden, da ich durch diese * und ** (Pointer auf einen Pointer?) vorne und hinten irgendwie verwirrt bin. Da fehlt mir die Grundlage. Aber das Prinzip habe ich verstanden.

Ich glaube, dass ich jetzt speichertechnisch in dem Abschnitt auf der sicheren Seite bin.

Die MySQL-Zeileninhalte wird per atoi() und strclone() kopiert und dann an den asynchronen Thread gegeben. Der Thread widerum kombiniert per strcombine() die übergebene URL, um etwas anzuhängen.

Ich habe folgende Hilfsfunktion eingeführt:

static char* strcombine(const char* str1, const char* str2) {
	#ifndef __cplusplus
	char *newstr = malloc(strlen(str1)+strlen(str2)+1);
	#else
	char *newstr = (char*)malloc(strlen(str1)+strlen(str2)+1);
	#endif

	memcpy(newstr, str1, strlen(str1)+1);
	strcat(newstr, str2); // Wäre memcpy() nicht sicherer?

	return newstr;
}

Mit den Freigaben am Ende von meine_funktion():

char* completeurl = strcombine(k->url, URL_APPENDIX);
.
.
.
free(completeurl); // malloc() von strcombine() wieder freigeben
free(k->url); // malloc() von strclone() wieder freigeben
free(k); // malloc() von mysql_abfrage() wieder freigeben

Ich verwende memcpy() anstelle von strcpy() um vor Pufferüberläufen geschützt zu sein. (Dies wurde mir mal in einer Mailing-List strengstens geraten).

Gibt es bezüglich dem anschließenden Verwenden von strcat() Bedenken bezüglich der Sicherheit?

Ich glaube, dass ich nun dieses Speicherzugriffs-Problem behoben habe (denn die Meldungen kommen ab jetzt etwas später), aber irgendwie scheinen andere Programmteile immer noch weitere Fehler zu haben, bei denen ich keine bedenklichen Stellen finde. Oder ist jetzt an diesem aktuellen Code noch etwas unsauber, sodass sich ein Fehler in späteren Programmcode auswirken kann?

Gruß
blackdrake

[namespace invader]

blackdrake schrieb:

Ich verwende memcpy() anstelle von strcpy() um vor Pufferüberläufen geschützt zu sein. (Dies wurde mir mal in einer Mailing-List strengstens geraten).

Gibt es bezüglich dem anschließenden Verwenden von strcat() Bedenken bezüglich der Sicherheit?

strcpy() und strcat() sind nur unsicher, wenn man blind beliebig große Strings in einen festen Puffer (also char newstr[100]) kopiert. Aber um das zu vermeiden verwendest du ja keinen festen Puffer, sondern einen, der genau so groß ist wie er sein muss. Du kannst schon darauf vertrauen, dass strlen() richtig funktioniert.

Oder ist jetzt an diesem aktuellen Code noch etwas unsauber, sodass sich ein Fehler in späteren Programmcode auswirken kann?

Ich sehe jetzt keine Probleme mehr (abgesehen davon, dass keine Fehler von malloc und den pthread-Funktionen abgefangen werden).

[blackdrake]

Hallo.

namespace invader schrieb:

Ich sehe jetzt keine Probleme mehr

Mh, dann muss ich wohl noch länger an dem Speicherfehler suchen. Ich suche jetzt schon seit fast über einer Woche ... kann doch irgendwie nicht so schwer sein bei nur 500 Zeilen Code...

namespace invader schrieb:

(abgesehen davon, dass keine Fehler von malloc und den pthread-Funktionen abgefangen werden).

Wie sollte ich die Fehlerfälle überprüfen?

Bei dem Thread weiß ich nicht, wie ich den Erfolgsfall überprüfen soll. Oder meinst du gar, Exceptions abfangen?

Bei malloc() habe ich gelesen, dass man das Ergebnis auf NULL überprüfen sollte.

Aber mal eine ganz simple frage: Was soll ich dann machen? Schlägt malloc() fehl, ist der Arbeitsspeicher des Endanwendersystems voll. Es ist ja quasi "alles im Eimer", da man in dem Fall nicht mal die einfachsten Dinge machen kann.

Mir fallen da 2 Dinge ein:
1. Warnung in Logdatei ausgeben* und Programm per exit() verlassen.
2. Warnung in Logdatei ausgeben* und die Funktion per return false; verlassen. In dem Falle führt der Daemon seine Arbeit fort, wenn der Arbeitsspeicher wieder platz hat.

* = das ist bedenklich, denn wenn der Arbeitsspeicher voll ist, wie soll man da noch ein Filehandle und einen Puffer für fprintf() eröffnen können?

Gruß
blackdrake

[namespace invader]

blackdrake schrieb:

Wie sollte ich die Fehlerfälle überprüfen?

Bei dem Thread weiß ich nicht, wie ich den Erfolgsfall überprüfen soll. Oder meinst du gar, Exceptions abfangen?

Die pthread-Funktionen geben meistens ein int zurück, das den Fehler anzeigt. Steht in den man pages.

Bei malloc() habe ich gelesen, dass man das Ergebnis auf NULL überprüfen sollte.

Aber mal eine ganz simple frage: Was soll ich dann machen? Schlägt malloc() fehl, ist der Arbeitsspeicher des Endanwendersystems voll. Es ist ja quasi "alles im Eimer", da man in dem Fall nicht mal die einfachsten Dinge machen kann.

Trotzdem sollte ein standardkonformes Programm sich ordentlich mit exit(EXIT_FAILURE) beenden, als einfach den NULL-Pointer zu verwenden und damit einen Speicherzugriffsfehler oder etwas beliebig anderes (nasal demons...) zu verursachen.

Praktisch wird malloc selten bis niemals fehlschlagen, eher wird das Programm durch den OOM Killer beendet oder ähnliches. Aber man weiß ja nie.

* = das ist bedenklich, denn wenn der Arbeitsspeicher voll ist, wie soll man da noch ein Filehandle und einen Puffer für fprintf() eröffnen können?

Man kann es zumindest versuchen. Eventuell ist malloc ja nur fehlgeschlagen, weil ein viel zu großer Speicherbereich angefordert wurde.

[blackdrake]

Ach, statt meiner selbstgebastelten Funktion strclone() hätte ich ja auch strdup() nehmen können.

Habe nun alle Threads- und Malloc-Fehler abgefangen. Ich beende das Programm aber nicht, sondern verlasse die Methode, in Hoffnung, dass bei einem RAM-Voll-Problem die Anwendung wieder weiterläuft, sobald wieder Platz ist.

Das ganze ist in folgendem Stil gemacht:

static bool _pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void*), void *arg) {
	int rc = pthread_create(thread, attr, start_routine, arg);

	if (rc != 0) {
		char bfr[BFR_SIZE];
		snprintf(bfr, BFR_SIZE, "Could not create thread. Error-Code: '%d'", rc);
		log_event(VERBOSE_LEVEL_ERROR, bfr);
	}

	return rc == 0;
}

Meine anderen Funktionen:

static char* strcombine(const char* str1, const char* str2) {
	#ifndef __cplusplus
	char *newstr = malloc(strlen(str1)+strlen(str2)+1);
	#else
	char *newstr = (char*) malloc(strlen(str1)+strlen(str2)+1);
	#endif

	if (newstr == NULL) {
		log_event(VERBOSE_LEVEL_ERROR, "Malloc-Error at strcombine()!");
		return NULL;
	}

	memcpy(newstr, str1, strlen(str1)+1);
	strcat(newstr, str2);

	return newstr;
}

static char* strclone(const char* str) {
	#ifndef __cplusplus
	char *newstr = malloc(strlen(str)+1);
	#else
	char *newstr = (char*)malloc(strlen(str)+1);
	#endif

	if (newstr == NULL) {
		log_event(VERBOSE_LEVEL_ERROR, "Malloc-Error at strclone()!");
		return NULL;
	}

	memcpy(newstr, str, strlen(str)+1);

	return newstr;
}

Gruß
blackdrake