Probleme mit select() in einem (simplem) p2p programm in udp

Heyho,

ich habe ein kleines problem mit meinem p2p programm, welches musikdaten zwischen 3-10 personen senden können soll. die datenübertragung funktioniert soweit einwandfrei, wobei aber ich das problem habe, dass nur der erste client empfängt und die anderen nur senden.
nach kurzem nachforschen habe ich herausgefunden, dass für jeden schleifendurchlauf der erste client IMMER 1 in select erhält und die anderen beiden immer 0.

hier der sendeabschnitt:

case 6:
{
    if(!filename.empty())
    {
        time_t start, now;
        time(&start);
        double elapsedtime = 0;
        char *recvbuf;
        char *sendbuf;
        recvbuf = new char[1024];
        sendbuf = new char[1024];
        int recvfd, sendfd,err, port;
        int errormarker = 0;
        if((recvfd = socket(AF_INET,SOCK_DGRAM,0)) == -1)
        {
            std::cout << "Error setting socket\n";
            errormarker =1;
        }
        if(errormarker ==1)
            break;
        struct sockaddr_in server_address;
        server_address.sin_family = AF_INET;
        server_address.sin_addr.s_addr = INADDR_ANY;
        server_address.sin_port = htons(50001);
        for (int i =0; i<=9;i++)
        {
            port = 50001+i;
            errormarker = bind(recvfd,(sockaddr*) &server_address, sizeof   (server_address));
            if(errormarker ==-1)
                server_address.sin_port = htons(server_address.sin_port+1);
            else
                break;
        }
        if(errormarker == -1)
        {
            std::cout << "all ports are used, please wait a minute and try again\n";
            break;
        }
        std::cout << "Searching for other musicdata owners\n";
        fd_set readset;
        while (elapsedtime < 15)
        {
            if(errormarker != 0)
                break;
            memset(recvbuf, 0, sizeof(recvbuf));
            memset(sendbuf, 0, sizeof(sendbuf));
            struct timeval tv;
            tv.tv_sec =0;
            tv.tv_usec = rnd(10000,50000);
            FD_ZERO(&readset);
            FD_SET(recvfd, &readset);
            int result = select(recvfd + 1, &readset, NULL, NULL, &tv);
            std::cout << result << "\n";
            if (result >0)
            {
                result = recvfrom(recvfd, recvbuf, 1024, NULL,NULL,NULL);
                if (result != -1)
                {
                    buftofile(recvbuf,filename);
                    addnewdata(filename);
                }
                else
                    std::cout << "error receiving data \n";
            }
            filetobuf(sendbuf,filename);
            for(int i = 50001; i<=50010;i++)
            {
                struct addrinfo hints, *servinfo, *p;
                memset(&hints, 0, sizeof hints);
                hints.ai_family = AF_UNSPEC;
                hints.ai_socktype =SOCK_DGRAM;
                std::string s_port = to_string(i);
                err = getaddrinfo(NULL,s_port.c_str(),&hints,&servinfo);
                for(p=servinfo;p!=NULL;p=p->ai_next)                                
                {
                    if(i == port)
                    {
                        err = sendto(recvfd, sendbuf,strlen(sendbuf), 0, p->ai_addr,p->ai_addrlen);
                        continue;
                    }
                    if((sendfd = socket(p->ai_family, p->ai_socktype,p->ai_protocol)) == -1)
                    {
                        continue;
                    }
                    else
                    {
                        err=sendto(sendfd,sendbuf,strlen(sendbuf), 0, p->ai_addr,p->ai_addrlen);
                        close(sendfd);                              
                    }
                }
                freeaddrinfo(servinfo);
            }
            time(&now);
            elapsedtime=difftime(now,start);
        }
        close(recvfd);

    }
    else
        std::cout << "Error, please choose a file to save your data first!" << std::endl;
}
break;

ich hab ehrlich gesagt keine ahnung, warum das so sein könnte/kann keinen direkten fehler erkennen. nach meiner theorie sollten sich alle abwechseln und jeder mal empfangen.

vielen dank für die hilfe im voraus.

Shadowofsora schrieb:

case 6:
        ...
        char *recvbuf;
        char *sendbuf;
        recvbuf = new char[1024];
        sendbuf = new char[1024];
                ...
                struct addrinfo hints, *servinfo, *p;
        ...
}
break;

Komische Mischung aus C und C++. Mindestens in eine eigene Funktion hättest du das Ding verschieben können.

Zum Problem: Lesen etwa alle vom gleiche Socket?

Ausserdem ist udp je nach Anwendung wohl nicht die richtige Ebene.

es ist eine hausaufgabe für die uni und udp war leider die vorgabe.
wir hatten an sich nicht so viel c++ bisher und ich musste mich an c und c++ guides belesen, weswegen ich wahrscheinlich eine bunte mischung habe. was genau daran ist c-lastig? bzw hätte ich anders machen können?

//sorry für den doppelpost//

ich habe 2 sockets 1 binded sich an einen der 10 ports und soll empfangen und der andere wird zum senden immer wieder neu initialisiert. select nutze ich um zu erkennen, wann denn ein client etwas empfangen kann, damit ich nicht in einen endlosen recvfrom Blocking-mode komme.

SeppJ

shadowofsora schrieb:

was genau daran ist c-lastig? bzw hätte ich anders machen können?

Im Prinzip ist es nicht direkt C-lastig, aber es ist eben der typische Stil von schlechten C-Büchern, in dem fleißig mit rohen Pointern und und händisch mit dynamischem Speicher hantiert wird. Im Prinzip sollte man das so auch nicht in C machen, viele C-Programmierer kommen darüber jedoch nicht hinaus, deshalb wird das gerne mit C verbunden. In C++ ist es jedoch sehr, sehr einfach, diese extrem fehleranfällige Programmierweise zu vermeiden, indem man einfach die Standardbibliothek benutzt:
Je nachdem, ob eher eine Zeichenkette oder eine Folge von chars gemeint ist:

string recvbuf(1024, 0);
// oder
vector<char> recvbuf(1024);

Wobei in letzterem Fall char vielleicht nicht der beste Typ ist und man evtl. eher signed oder unsigned char möchte.

Ähnlich könnte man deinen Code an vielen Stellen verbessern, aber ich werde jetzt nicht 100 Zeilen komplett überarbeiten.

ich würde auch nicht erwarten, dass du meinen code in irgenteiner weise optimierst/verbesserst :), aber vielen dank für den hinweis, ich habe c so ziemlich in der vorlesung nur mit "rohen" pointern gelernt und dachte, dass dies der standard dafür sei ~ muss ich mich wohl irgentwann mal etwas damit auseinandersetzen. Wenn ich denn jetzt string recvbuff(1024,0) nehmen würde könnte ich den dann in sendto als buffer nehmen, in dem ich einfach recvbuff.c_str() nehmen würde?

und damit wieder vom offtopic zum hauptthema, dem select bzw eine andere methode mein ziel zu erfüllen.

SeppJ

shadowofsora schrieb:

ich würde auch nicht erwarten, dass du meinen code in irgenteiner weise optimierst/verbesserst :), aber vielen dank für den hinweis, ich habe c so ziemlich in der vorlesung nur mit "rohen" pointern gelernt und dachte, dass dies der standard dafür sei ~ muss ich mich wohl irgentwann mal etwas damit auseinandersetzen. Wenn ich denn jetzt string recvbuff(1024,0) nehmen würde könnte ich den dann in sendto als buffer nehmen, in dem ich einfach recvbuff.c_str() nehmen würde?

Jain. Für das Senden ja, für das Empfangen natürlich nicht (weil const-qualifiziert). Aber c_str ist sematisch nicht das, was du suchst, auch wenn es technisch gesehen vielleicht funktioniert. c_str ist für eine Darstellung des Stringinhalts als eine Zeichenkette im C-Stil (nullterminiertes char-Array), aber send und recv arbeiten nicht mit Zeichenketten (daher fände ich vector<char> auch passender), sondern mit Bytefolgen (auch wenn dahinter der gleiche Datentyp steht). Die data-Methode wäre daher passender (im Falle von string ist diese ebenfalls const-qualifiziert, bei vector nicht, mit entsprechenden Folgen für die Eignung als recv-Buffer), die einem direkten Zugriff auf die unterliegende Bytefolge gibt. Oder aber einfach die Adresse des ersten Elements nehmen (so bekommt man auch bei string einen Schreibzugriff auf die unterliegende Bytefolge).

und damit wieder vom offtopic zum hauptthema, dem select bzw eine andere methode mein ziel zu erfüllen.

Darüber muss ich erst einmal noch genauer nachdenken.

ich kann bei bedarf auch das gesammte programm einfügen

SeppJ

shadowofsora schrieb:

ich kann bei bedarf auch das gesammte programm einfügen

Das beste ist immer ein minimales, lauffähiges Beispiel. Das heißt, du kürzt wirklich aggressiv alles außer dem Code, der den Fehler verursacht. Das ist durchaus einiges an Arbeit, wenn du es richtig machst (aber du suchst ja schließlich Hilfestellung), hat aber auch den großen Vorteil, dass man dabei oft selber den Fehler findet. Denn entweder merkt man beim Kürzen des Codes, dass der Fehler eigentlich ganz woanders war als man dachte (wenn er plötzlich weg ist, wenn man etwas vermeintlich Irrelevantes kürzt) oder weil der Fehler offensichtlich wird, wenn man nur noch 10-20 Zeilen Code statt 100-200 vor sich hat.
Und falls man den Fehler dabei selber nicht finden sollte, macht man es erfahrenen Fehlersuchern trotzdem wesentlich leichter.

bin mir nicht sicher ob das minimal ist, aber ich hab soviel entfernt, wie cih mir gedacht habe, raus muss, damit mein grundprinzip erhalten bleibt.
Ich habe die konkrete dateiverarbeitung entfernt (alles schon geprüft) und natürlich alles drumherum für den "user". sind aber immernoch 128 zeilen :3

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <unistd.h>
#include <iostream>
#include <string>
#include <cstring>
#include <limits>
#include <fstream>
#include <time.h>
#include <netdb.h>
#include<sstream>

std::string to_string(int value)
{
//create an output string stream
	std::ostringstream os ;
//throw the value into the string stream
	os << value ;
//convert the string stream into a string and return
	return os.str() ;
}

int rnd(int start, int range)
{
	srand (time(NULL));
	return rand() % range + start;
}

int main()
{

	std::fstream fs;
	std::string filename, tmpline;
	filename = "test";
	time_t start, now;
	time(&start);
	double elapsedtime = 0;
	char *recvbuf;
	char *sendbuf;
	recvbuf = new char[1024];
	sendbuf = new char[1024];
	int recvfd, sendfd,err, port;
	int errormarker = 0;
	if((recvfd = socket(AF_INET,SOCK_DGRAM,0)) == -1)
	{
		std::cout << "Error setting socket\n";
		errormarker =1;			
	}
	struct sockaddr_in server_address;
	server_address.sin_family = AF_INET;
	server_address.sin_addr.s_addr = INADDR_ANY;
	server_address.sin_port = htons(50001);
	for (int i =0; i<=9;i++)
	{
		port = 50001+i;
		errormarker = bind(recvfd,(sockaddr*) &server_address, sizeof	(server_address));
		if(errormarker ==-1)
			server_address.sin_port = htons(server_address.sin_port+1);
		else
			break;
	}
	if(errormarker == -1)
	{
		std::cout << "all ports are used, please wait a minute and try again\n";
	}
	std::cout << "Searching for other musicdata owners\n";
	fd_set readset;
	while (elapsedtime < 15)
	{
		if(errormarker != 0)
			break;
		memset(recvbuf, 0, sizeof(recvbuf));
		memset(sendbuf, 0, sizeof(sendbuf));
		struct timeval tv;
		tv.tv_sec =0;
		tv.tv_usec = rnd(10000,50000);
		FD_ZERO(&readset);
   		FD_SET(recvfd, &readset);
   		int result = select(recvfd + 1, &readset, NULL, NULL, &tv);
		std::cout << result << "\n";
		if (result >0)
		{
			result = recvfrom(recvfd, recvbuf, 1024, NULL,NULL,NULL);
			if (result != -1)
			{
				fs.open("test",  std::fstream::in | std::fstream::out | std::fstream::app);
				fs << recvbuf << "\n";
				fs.close();
			}
			else
				std::cout << "error receiving data \n";
		}
		for(int i = 50001; i<=50010;i++)
		{
			std::cout << "for\n";
			struct addrinfo hints, *servinfo, *p;
			memset(&hints, 0, sizeof hints);
			hints.ai_family = AF_UNSPEC;
			hints.ai_socktype =SOCK_DGRAM;
			std::string s_port = to_string(i);
			err = getaddrinfo(NULL,s_port.c_str(),&hints,&servinfo);
			for(p=servinfo;p!=NULL;p=p->ai_next)								
			{
				if(i == port)
				{
					err = sendto(recvfd, "Hello from Client 1",20, 0, p->ai_addr,p->ai_addrlen);
					continue;
				}
				if((sendfd = socket(p->ai_family, p->ai_socktype,p->ai_protocol)) == -1)
				{
					continue;
				}
				else
				{
					err=sendto(sendfd,"Hello from Client 1",20, 0, p->ai_addr,p->ai_addrlen);
					close(sendfd);
				}
			}
						freeaddrinfo(servinfo);
		}
		time(&now);
		elapsedtime=difftime(now,start);
	}
	close(recvfd);				
}

lustigerweise ist immer noch genau der selbe fehler drinne.
man erhält jetzt im ersten client (ab jetzt müssen sie in verschiedenen ordnern sein bzw verschiedene dateien existieren mit anderen ordnernamen)
so 30-50? anchrichten, währenddessen die beiden andern nicht mal dazu kommen die datei zu erstellen (tun sie ja auch jetzt nur, wenn sie was empfangen).

nochmal zum grundansatz: ich nutze an sich select nur, damit ich sagen kann, dass jeder client zwischen 0,1-0,6 sekunden versucht etwas zu empfangen und dann anfängt zu senden (in einer schleife) für x sekunden.
select könnte hier natürlich der falsche ansatz gewesen sein, aber es war das einzige passende, was ich jetzt konkret gefunden hatte.

ich werd jetzt wohl einfach mal probieren, ob es mit poll() statt select funktioniert

poll ruft genau den selben fehler hervor wie select --- also muss der fehler irgentwo in der (eigetnlich fehlerfreifen) socketerstellung sein? hab nochmal die ports geprüft, jeder client krieg wie hervorgesehen einen passenden, das programm sollte automatisch stoppen, wenn der recvfd nicht initalisiert werden kann und mehr probleme kann ich mir jetzt nicht direkt vorstellen --- ich hab keine ahnung was da falsch sein soll.

ich hab den fehler zwischendurch auch in meinem "sendto" abschnitt gesucht, hab dabei herausgefunden, dass getaddrinfo mir für jeden port genau 2 addrinfo structs gibt (ich hatte zuerst erwartet, dass es bei manchen sofort NULL sein würde und deswegen alle ports nach dem 1. vllt einfach ignoriert werden würden),
bedauerlicherweise konnte ich dort den fehler auch nicht direkt finden