Tearing my hair out.

Hey guys, I'm trying to code a function that binds on a port, then listens for incoming UDP packets containing a certain string. Each time it receives that string in a packet, it logs the sender's IP address in a string array, it then prints out the contents of that array once it is full. The problem is, when I print it I see that it is full of dublicates of the last entry rather than the list of the multiple hosts that it has received the packet from. I have no idea why it is doing this, could it be something to do with the way I am testing it? (I am using hping to send the packets using the -a option to spoof the source address).

My code is as follows, try to compile it and you'll see what I mean.

Code:

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h> 
#include <netdb.h>
#include <unistd.h>
main(){
	int sockfd;
	struct sockaddr_in their_addr;
	struct sockaddr_in my_addr;
	char *addresses[5];
	int address_length = sizeof(struct sockaddr);
	if ((sockfd = socket(AF_INET, SOCK_DGRAM, 0)) == -1) {
        perror("socket");
        exit(1);
    }
	
	bzero((char *) &my_addr, sizeof(my_addr));	
	my_addr.sin_family = AF_INET;
	my_addr.sin_addr.s_addr = htonl(INADDR_ANY);
	my_addr.sin_port = htons(9999);
	
	if ( (bind(sockfd, (struct sockaddr *) &my_addr, sizeof(my_addr)) < 0)){
		perror("Bind");
		}
	int sp = 1;
	char recvd[100];
	while(sp<=5){	
		
		int bytes_recvd = recvfrom(sockfd, recvd, 5, 0, (struct sockaddr *)&their_addr, &address_length);
		
		if (bytes_recvd == -1){
			perror("Recvfrom");
			exit(1);
		}
		
		else if(bytes_recvd > 0){
			if (strncmp(recvd,"ackbr",5) == 0){
				printf("%d\n", sp);
				addresses[sp] = inet_ntoa(their_addr.sin_addr);
				sp++;
			}
		}
	}
	int x = 1;
	while(x<=5){
		printf("queue[%d] = %s\n", x++, addresses[x]);
	}
		
								
}

Thanks for your time. There's probably a more elegant way of doing this anyway.

addresses[sp] = inet_ntoa(their_addr.sin_addr);

You're not making a copy of the hostname here. From the man page:

inet_ntoa() returns the dots-and-numbers string in a static buffer that is overwritten with each call to the function.

In other words, try making a copy of the string.

[edit] Might I suggest:

Code:

const char *name = inet_ntoa(their_addr.sin_addr);
addresses[sp] = malloc(strlen(name) + 1);
if(!addresses[sp]) {
    perror("out of memory: malloc()");
    /* ... */
}
strcpy(addresses[sp], name);

/* ... */
free(addresses[x]);

[/edit]

I Am Not A Network Programmer, but my hat of guessing says that this line:

Code:

addresses[sp] = inet_ntoa(their_addr.sin_addr);

sets the pointer addresses[sp] to point to changing data (so that each of your pointers ends up being the same, so they all print the same thing) instead of doing

Code:

addresses[sp] = malloc(16); //I think that's enough for an IP address
strcpy(addresses[sp], their_addr.sin_addr);

This assumes that (edit) inet_ntoa returns a character array/pointer, which I didn't bother to look up. Edit: and of course I meant inet_ntoa(their_addr.sin_addr) in the code thing too, because I can read, really I can.

Yeah, "xxx.xxx.xxx.xxx\0" is 16 chars. Why didn't I think of that?

But then . . .

Code:

char addresses[5][16];

Thanks for the help guys, making a copy of the hostname works nicely. I need to read more documentation in future.

May I point out two things?
First: http://cpwiki.sourceforge.net/Implicit_main
Second: Your indentation is messed up. Avoid indenting with both spaces and tabs. Choose one.