Linux Socket: Datei in einem Paket verschicken

fukurokuju

Hallo Community,

ich hänge schon seit einigen Tagen bei der Versendung eines Paketes fest, dass unter anderem eine Datei von einem Server zu einem Client schicken soll. In meinem Programm gibt es ein Paket (JOB_DESCRIPTION), das so aussieht:

struct JOB_DESCRIPTION
	{
	 	uint jobId;
		char documentFileName[100];
		char documentName[100];
		char agent[100]; // should be changed
		char arguments[100]; // list of arguments passed to agent
		bool ret; // indicates, whether document has to be returned to server after updating it
		STATUS status;
		void *next;
		char getDate[10];
		char lastModi[10];
		uint fileSize;
		void *file;
	};

	struct PACKET_HEADER
	{
		PACKET_TYPE packetType;
		uint payloadSize;	
	};

	struct PACKET
	{
		PACKET_HEADER header;
		void *payload;
	};

Auf Serverseite wird diese Struktur nun gefüllt und Richtung Client gesendet:

DMSClient::PACKET jobData;
	DMSClient::JOB_DESCRIPTION jobDescription;

	// Create packet
	jobData.header.packetType = DMSClient::SEND_JOB;
	jobDescription.jobId = 152;
	memcpy(&jobDescription.documentFileName, "testfile", sizeof("testfile"));
	// READ FILE
	string filebuffer;
	readBinaryFile(jobDescription.documentFileName, filebuffer);
	memcpy(&jobDescription.documentName, "file", sizeof("file"));
	memcpy(&jobDescription.agent, "shell", sizeof("shell"));
	memcpy(&jobDescription.arguments, "open", sizeof("open"));
	jobDescription.ret = true;
	jobDescription.status = DMSClient::STORE;
	jobDescription.next = NULL;
	jobDescription.fileSize = filebuffer.length();
	memcpy(&jobDescription.getDate, "111111", sizeof("111111"));
	memcpy(&jobDescription.arguments, "222222", sizeof("222222"));

	jobData.header.payloadSize = sizeof(jobDescription) + filebuffer.length();

	// reserve memory for file
	jobDescription.file = (char *)malloc(filebuffer.length());
	if (jobDescription.file == NULL) cout << "malloc error" << endl;

	jobData.payload = &jobDescription;
	// create send buffer
	memcpy(jobDescription.file, filebuffer.c_str(), filebuffer.length());
	memcpy(dataBuffer, &jobData.header, HEADER_SIZE);
	memcpy(dataBuffer + HEADER_SIZE, &jobDescription, sizeof(jobDescription));
	memcpy(&(((DMSClient::JOB_DESCRIPTION *)(dataBuffer + HEADER_SIZE))->file), &jobDescription.file,  filebuffer.length());

        int r = server.send(dataBuffer, HEADER_SIZE + sizeof(DMSClient::JOB_DESCRIPTION) + filebuffer.length() + 500, socketFd);

Ich lese mit der Funktion readBinaryFile() die Datei ein und fülle dann die Struktur mit den gewünschten Daten.
Auf der Clientseite kommt das Paket auch an. Ausgelesen wird es dort so:

try
	{
		int h = threadSocket.recv(buffer, MAXSOCKBUFFER, 0);
		cout << "[thread] Received job-data from server. Bytes:" << h << endl;
	}
	catch (DMSClient::SocketException e)
	{
		cout << "SOCKET EXCEPTION: " << e.getError() << endl;
	}
	jobData = (DMSClient::PACKET *)buffer;
	jobDescription = (DMSClient::JOB_DESCRIPTION *)&(jobData->payload);

	cout << "_______________________" << endl;
	cout << "______PACKET DATA______" << endl;
	cout << "_______________________" << endl;

	cout << "__[HEADER]__" << endl;
	cout << "Packettype: " << jobData->header.packetType << endl;
	cout << "PayloadSize:  " << jobData->header.payloadSize << endl;

	cout << "[__PAYLOAD__]" << endl;
	cout << "JobId: " << jobDescription->jobId << endl;
	cout << "DocumentFilename: " << jobDescription->documentFileName << endl;
	cout << "DocumentName: " << jobDescription->documentName << endl;
	cout << "Agent: " << jobDescription->agent << endl;
	cout << "Arguments: " << jobDescription->arguments << endl;
	cout << "Return: " << jobDescription->ret << endl;
	cout << "Status: " << jobDescription->status << endl;
	cout << "Filesize " << jobDescription->fileSize << endl;
	cout << "File: " << (char *) jobDescription->file;

Folgendes Problem: ALLE Daten, bist auf jobDescription->file kommen vernuenftig an! In jedem Feld steht der Wert, den ich auf der Serverseite in das Paket reingeschickt habe, nur in File steht Müll. Ich weiss nicht, wo ich den Fehler mache. Auf Serverseite habe ich alle Daten nach Zusammenstellung des Pakets nochmal ausgegeben lassen. Dort wird alles richtig zusammengebaut.
Habt ihr vielleicht eine Idee?
Danke im Voraus.

Gruß fuku

du verschickst nur einen pointer auf den dateinamen und nicht den namen selbst. du musst das so wie bei documentFileName zb machen. am server liest er den pointer korrekt aus, aber der zeigt dann irgendwo hin in den speicherraum des server prozesses und gib das, was dort steht, aus.

bedenke noch beim übertragen von structs, dass das sehr vom abi (nicht api) abhängt, da diese das binäre layout der structs definiert. außerdem solltest du dir gedanken über charsets in deinen strings machen. aber das nur am rande.

fukurokuju

Ersteinmal vielen Dank für die schnelle Antwort. Du sprichst also diese Zeile an:

memcpy(&(((DMSClient::JOB_DESCRIPTION *)(dataBuffer + HEADER_SIZE))->file), &jobDescription.file,  filebuffer.length());

?

Daran hatte ich versucht extra zu denken, darum habe ich ja auch ein "&" for "jobDescription.file" geschrieben. Greife ich nicht so auf die Adresse zu an der der Pointer liegt? Und dort liegt doch auch meine Datei!?

jetzt hab ich massenhaft zeug geschrieben und diese dumme forum lässt mich es nicht senden. es meint, es wäre spam. dann lös ich diese matheaufgabe und er jammert, dass ich ein thema angeben soll. dann geb ich eines an und er jammert IMMER NOCH, dass ich ein thema angeben soll! zum teufel (und das sag ich als agnostiker), wie soll man leuten helfen, wenn man hier derart von der software schikaniert wird?

fukurokuju: gib deine mail adresse an oder so was, dann schick ich es dir direkt. hier geht s ja nicht.

nman

namenlos schrieb:

wie soll man leuten helfen, wenn man hier derart von der software schikaniert wird?

Registrier Dich einfach, das Problem ist bekannt und steht in der Liste der zu behebenden Bugs wohl recht weit oben.

nach mehreren versuchen mit dem captcha bin ich endlich hier... ihr könnt auch gleich den bug fixen, dass das passwort zu lang ist. er akzeptiert es beim registrieren und schick es per mail, akzeptiert es beim login dann aber nicht. 32 zeichen is schon zu lang.

mein beitrag
-------
hmm mal überlegen, wie ich das am besten erklär.

ein pointer zeigt auf daten, ist aber selbst ein datum (singular von daten, keine zeitangabe). malloc reserviert dir auf dem heap eine menge zusammenhängender bytes und gibt die adresse diese stückes zurück. die größe eines pointers ist abhängig von der architektur und beträgt bei 32 bit systemen 4 byte, bei 64 bit 8 byte.

du allokierst zuerst speicher im client programm für den inhalt der datei (der aber eh schon in filebuffer steht. das könntest du dir also theoretisch sparen, ist aber nicht unbedingt jetzt das große problem.). wichtig ist, dass dieser speicherbereich dynamisch zur laufzeit erzeugt wird und das auch nur am client. der server weiß davon gar nichts. du solltest übrigens lieber new/delete anstelle von malloc/free verwenden, da du ja c++ programmierst. das is aber auch nur eine feinheit.

memcpy(jobDescription.file, filebuffer.c_str(), filebuffer.length());

diese zeile kopiert dann in den mit malloc reservierten speicherbereich die daten der datei.
dann beginnst du "databuffer" zu füllen. ich hab in deinem codeausschnitt keine variable gefunden, die so heißt. ich nehm also an, dass das ein char array ist, dessen länge ich nicht kenne. du kopierst zuerst den paket header und dann die payload rein. das is auch soweit richtig, weshalb fast alles funktioniert.
dann willst du den inhalt der datei in den "databuffer" kopieren. das ist grundsätzlich die richtige idee, nur etwas falsch umgesetzt. du kopierst jetzt im endeffekt teile des stacks in deinen "databuffer", da du von "jobDescription.file" die adresse nimmst. du kopierst damit jetzt den wert von "jobDescription.file" (was die adresse des speichers von malloc ist.) inklusive andere daten vom stack, da "jobDescription" am stack allokiert wurde. dh, du kopierst nicht die daten der datei. um das zu tun, müsstest du das "&" weglassen. dann würde zumindest das richtige kopiert werden. aber noch nicht an die richtige stelle.
"jobDescription.file" wird daten unterschiedlicher länge enthalten. dazu muss es als char array mit einem element definiert werden. das ist die beste lösung.

struct JOB_DESCRIPTION
     {
          uint jobId;
         char documentFileName[100];
         char documentName[100];
         char agent[100]; // should be changed
         char arguments[100]; // list of arguments passed to agent
         bool ret; // indicates, whether document has to be returned to server after updating it
         STATUS status;
         void *next;
         char getDate[10];
         char lastModi[10];
         uint fileSize;
         char file[1];
     };

0 ist nicht erlaubt in c++. die größe des arrays ist auch nicht wichtig, da sie unbekannt ist zur compilierzeit. es geht nur darum, dass "file" vom typ char array ist.
die zeile müsste dann also richtig so lauten:

memcpy((((DMSClient::JOB_DESCRIPTION*)(dataBuffer + HEADER_SIZE))->file), jobDescription.file, filebuffer.length());

nachdem ich aber nicht deinen ganz code kennen und der teilweise _abenteuerlich_ ist, kann ich dir das so nicht garantieren. sehr optimal ist diese lösung auch noch nicht. die zeile zum senden sollte dann so aussehen:

int r = server.send(dataBuffer, HEADER_SIZE + sizeof(DMSClient::JOB_DESCRIPTION) + filebuffer.length() + 499, socketFd);

wichtig ist hier, dass nicht mehr 500 (woher auch immer das kommt...) sondern 499 dazugezählt wird, da du im "sizeof(DMSClient::JOB_DESCRIPTION)" ja schon ein byte von deinem ein-byte-array "file" drin hast und dieses deshalb von "filebuffer.length()" abziehen musst.

nman

namenlos schrieb:

nach mehreren versuchen mit dem captcha bin ich endlich hier...

Cool, willkommen an Bord, mir sind Deine Beiträge in letzter Zeit schon positiv aufgefallen.

ihr könnt auch gleich den bug fixen, dass das passwort zu lang ist. er akzeptiert es beim registrieren und schick es per mail, akzeptiert es beim login dann aber nicht. 32 zeichen is schon zu lang.

Erstellst Du dafür bitte einen Thread in der Forentechnik? Ich bin hier für den Server zuständig, nicht für den Forencode.

fukurokuju

Hi "namenlos".
Nochmal danke dafür, dass du dir die Zeit nimmst mir zu helfen.
Ich gehe mal auf Stück für Stück auf deine Antwort ein:

um das zu tun, müsstest du das "&" weglassen. dann würde zumindest das richtige kopiert werden. aber noch nicht an die richtige stelle.

Verstehe ich generell. Ohne Ampersand kopiere ich das, worauf file jobDescription zeigt.
Bevor ich es vergesse: Die Variable, die du gesucht hast, heißt "dataBuffer" und die Deklaration ist weiter oben im Quelltext. Hatte das aus Gründen der Übersichtlichkeit nicht auch noch angegeben. Du hast Recht, sie ist als char Array definiert.
Jetzt zu dem Teil deiner Antwort, den ich gar nicht begreife:

aber noch nicht an die richtige stelle.
"jobDescription.file" wird daten unterschiedlicher länge enthalten. dazu muss es als char array mit einem element definiert werden. das ist die beste lösung.

Dass jobDescription.file Daten unterschiedlicher Länge enthalten wird stimmt selbstverständlich. Aber warum muss es deshalb als char array mit einem Element definiert werden? Ich habe doch vorher per malloc doch Speicher in Höhe der Dateigröße definiert!? Warum also wird der Dateiinhalt nicht an die richtige Stelle kopiert?
Die 500, die da im Quelltext auftauchte, stammt noch aus Frust-Debug Versuchen, genauso wie sonstiger unsinnger Quatsch (Das Anlegen eines extra Buffers für fileBuffer z.B.) und wird entrümpelt in der Produktivversion. Für deine Mühe erstmal dicken Dank!

stell dir die frage, warum du zb. "documentFileName" als char array mit 100 einträgen definierst. du willst ja daten übertragen. ein pointer zeigt aber nur auf diese daten. er IST nicht die daten. sozusagen...

vielleicht ein beispiel:

struct paket {
  int size;
  char data[1];
};

das binäre layout der daten könnte in etwa so aussehen:

|size|d|

wobei size hier beispielsweise 4 byte lang und d ein byte lang ist (padding ist hier nicht miteinberechnet, da irrelevant für die erklärung.) wenn du jetzt ein solches paket in einen buffer schreibst, sieht das ca. so aus:

buffer start -->|size|d|aaaaaaaaaaaaaaaaaten|<--buffer ende

dh, ganz am anfang steht deine struct und dann hast du noch platz übrig für daten, wobei das erste byte der daten schon teil der struct ist. das hat jetzt einen vorteil: du musst keine pointerarithmetik verwenden, um auf die daten zuzugreifen, da du einfach "data" der struct verwenden kannst, da das ja ein array vom typ char ist. "data[0]" zeigt noch in die struct, wobei "data[1]" schon auf das erste a im rest des buffers zeigt.
es gibt dann noch was zum thema padding zu sagen, aber das kommt erst später. zuerst muss dir klar sein, wie das mit der struct und dem char array funktioniert.

zugegeben ist "an die falsche stelle" schlecht ausgedrückt. du kopierst die daten an die richtige stelle, nur interpretierst du sie falsch dann. eben als pointer und nicht als daten.

fukurokuju

Ok, das verstehe ich soweit, allerdings:

Folgendes Beispiel-Struct:

struct DATA
{
int i;
char d[1];
int x;
};

Wenn hinter dem Char-Array - wie in deinem Beispiel - keine Daten mehr im Struct kommen, dann klappt das alles doch problemlos. Wenn aber Daten wie hier im Beispiel nach dem Daten-Char-Array kommen, die größer als 1 sind, dann überschreibe ich Teile des nächsten Feldes (hier int x). Richtig?!

Dann sagst du:

zugegeben ist "an die falsche stelle" schlecht ausgedrückt. du kopierst die daten an die richtige stelle, nur interpretierst du sie falsch dann. eben als pointer und nicht als daten.

In meinem Client interpretiere ich die Daten doch folgendermaßen:

jobData = (DMSClient::PACKET *)buffer;
    jobDescription = (DMSClient::JOB_DESCRIPTION *)&(jobData->payload);

    cout << "_______________________" << endl;
    cout << "______PACKET DATA______" << endl;
    cout << "_______________________" << endl;

    cout << "__[HEADER]__" << endl;
    cout << "Packettype: " << jobData->header.packetType << endl;
    cout << "PayloadSize:  " << jobData->header.payloadSize << endl;

    cout << "[__PAYLOAD__]" << endl;
    cout << "JobId: " << jobDescription->jobId << endl;

jobDescription zeigt hier extra NICHT auf die Adresse auf die jobData->payLoad zeigt, sondern auf die Stelle, an der der Pointer steht (darum das & davor). Ich raff' das alles nicht mehr.

Ich habe das Gefühl, dass meine ganze Protokollimplementation nichts taugt, und das zieht mich echt runter. Ist halt ein Projekt, dass ich für die Uni machen muss, und ich habe hier schon locker das dreifache der Zeit investiert, das eigentlich vorgesehen ist.
Sollte ich die Protokollimplementation neu aufziehen? Oder mir ein neues Studium suchen? Es ist doch zum Heulen...

fukurokuju schrieb:

Wenn aber Daten wie hier im Beispiel nach dem Daten-Char-Array kommen, die größer als 1 sind, dann überschreibe ich Teile des nächsten Feldes (hier int x). Richtig?!

ja, vollkommen richtig. deshalb stellt man die dynamisch großen daten ganz hinten hin.

fukurokuju schrieb:

In meinem Client interpretiere ich die Daten doch folgendermaßen:

in diesem codeausschnitt jetzt is auch nichts falsch. es funktioniert ja auch fast alles :-). es geht um das, was noch nicht funktioniert. "jobDescription.file" hat den falschen datentyp. du verschickst hier einen pointer. du musst aber daten verschicken. du verschickst im prinzip die ersten 4 byte der datei als pointer.

fukurokuju schrieb:

Ich habe das Gefühl, dass meine ganze Protokollimplementation nichts taugt, und das zieht mich echt runter. Sollte ich die Protokollimplementation neu aufziehen? Oder mir ein neues Studium suchen? Es ist doch zum Heulen...

also damit übertreibst du es dein protokoll ist nicht schlecht. nur eben dieser eine bug ist drin. hast du meine änderungsvorschläge mal ausprobiert? wenn nicht, dann probier mal und wir sehen weiter. niemand verlangt von dir, dass du alles beim ersten mal sofort perfekt machen musst... wenn du einige beispiele meiner studienkollegen ansehen würdest, wärst du echt schockiert und würdest dich für einen gott halten

fukurokuju

Auch auf die Gefahr hin, dass ich so langsam anfange zu nerven:

ja, vollkommen richtig. deshalb stellt man die dynamisch großen daten ganz hinten hin.

Gut, damit ist sichergestellt dass die Struktur sauber bleibt. Aber was wenn hinter dem struct Daten sind? Die werden dann trotzdem gnadenlos überschrieben, oda?

du nervst nicht. der sinn des forums is ja, dass man sich hilft.

die daten hinter der struct werden überschrieben. deshalb musst du einen buffer haben, der groß genug ist, um zuerst die struct und dann hinter der struct die daten aufzunehmen. wenn der buffer zu klein ist, hast du einen bug im programm und es werden möglicherweise wichtige andere daten überschrieben. ist der buffer groß genug (mindestens sizeof(struct) + dateilänge), wirst du keine probleme haben.

fukurokuju

So, Faxen dicke gehabt.. Nochmal den Quelltext aufgeräumt, und es läuft: Ich kann Dateien über das Netzwerk verschicken Für deine Hilfe erstmal ein dickes Dankeschön!

In einem deiner letzten Beiträge hast du folgendes geschrieben:

es gibt dann noch was zum thema padding zu sagen, aber das kommt erst später. zuerst muss dir klar sein, wie das mit der struct und dem char array funktioniert.

Von dem "Padding" habe ich schonmal was gehört. Wenn ich mich recht erinnere:
Padding bezeichnet das (automatische) Auffüllen von Strukturen durch den Compiler auf "runde" Bytes, damit Lese/Schreibzugriffe optimiert werden!?

Mal abgesehen davon, ob meine Aussage richtig ist: Inwiefern muss ich mich um dieses Padding kümmern, was kann passieren, und wie "fange" ich das sauber auf? Bzw. unter welchen Umständen tritt vll. gar kein Padding auf? Wie gesagt, das ist ein Projekt für die FH, und ich bin looooocker über dem geforderten Arbeitsaufwand, und muss so langsam mal runterschrauben, will aber auch keinen Schrott abgegeben.

[EDIT] Rechtschreibung...

fukurokuju schrieb:

Padding bezeichnet das (automatische) Auffüllen von Strukturen durch den Compiler auf "runde" Bytes, damit Lese/Schreibzugriffe optimiert werden!?

genau das ist es.

die lösung dazu ist etwas kompliziert. dazu brauchst du sizeof und offsetof. sizeof ist im prinzip klar. es gibt, angewandt auf eine struct deren größe in byte zurück. offsetof gibt angewandt auf eine struct und ein attribut dieser struct, die anzahl an byte zurück, die vor diesem attribut in der struct existieren:

struct X {
  int x;
  int y;
};
offsetof(X, y)

offsetof hat den wert von sizeof(int), da vor y das attribut x klarerweise die größe von int hat.

struct X {
  int x;
  char y[1];
};

bei diesem beispiel ist sizeof(X) wahrscheinlich nicht sizeof(int) + sizeof(char), sondern eher mehr. wieviel, weißt du nicht. wir wissen aber, dass offsetof(X, y) die anzahl an byte zurückgibt, die vor y stehen. wenn wir jetzt sizeof(X) - offsetof(X, y) verwenden, bekommen wir in summe die größe von y plus padding zurück. man könnte theoretisch auch sizeof(int) nehmen, da das ein einfaches beispiel ist.

struct X {
  int x1;
  int x2;
  char y[1];
};

hier ist das schon nerviger, weil du sizeof(int) gleich zwei mal nehmen musst. offsetof(X, y) ist aber genau das: sizeof(int) + sizeof(int). solltest du eine struct einmal ändern, fließt das in offsetof sofort ein. bei structs mit vielen attributen (zb. unterschiedlichen datentyps) ist es sowieso nervig, alle sizeof zusammenzuzählen. deshalb offsetof. damit du offsetof verwenden kannst, musst du für c stddef.h und für c++ cstddef includen.
ok. das zu offsetof

jetzt zu deinem fall. du allokierst databuffer von der größe sizeof(JOB_DESCRIPTION) + dateilänge - 1 (das kommt von char[1]). das is aber zu viel, da in sizeof(JOB_DESCRIPTION) auch das padding enthalten ist. deshalb ziehst du jetzt nicht mehr 1 ab, sondern (sizeof(JOB_DESCRIPTION) - offsetof(JOB_DESCRIPTION, file)). dann rechnet man herum und kommt zum ausdruck: offsetof(JOB_DESCRIPTION, file) + dateilänge. das -1 fällt weg, da das ja schon mit offsetof berücksichtigt wurde.