2

I'm writing a function that is supposed to do some operations and then return (using its arguments) the address of the device that is interacting with (i.e. that used sendto) the recvfrom inside the function.

Here's how I call the function, instantiating cliaddr before.

struct sockaddr_in cliaddr;
memset((void *) &cliaddr, 0, sizeof(cliaddr));
rcv_string(sockfd, &return_string, &cliaddr);
// Here I'll need to use cliaddr, that's why I need it outside too

Here's the implementation of the function:

int rcv_string(int sockfd, char **return_string, struct sockaddr_in *sndaddr) {
    // ...
    char buff[PACKETSZ + 2];
    memset(&buff, 0, sizeof(buff)); // Clean buffer
    socklen_t *plen = malloc(sizeof(struct sockaddr *));
    if ((recvfrom(sockfd, buff, sizeof(buff), 0, (struct sockaddr *) sndaddr, plen)) < 0) {
            perror("recvfrom");
            return -1;
    }
    char *snd_ip = malloc(INET_ADDRSTRLEN * sizeof(char));
    if (inet_ntop(AF_INET, &((*sndaddr).sin_addr.s_addr), snd_ip, INET_ADDRSTRLEN * sizeof(char)) == NULL) {
            perror("inet_ntop");
            return -1;
    }
    printf("Received '%s' from '%s'.\n", buff, snd_ip);
    // ...
}

Now, even if the ip address of the sending device is 192.168.1.251 I get the following output:

Received '0packetx' from '0.0.0.0'.

The received buffer is formatted correctly, but the address is evidently wrong. Why? Does it have to do with the definition of the address variable outside the function?

EDIT

If after the memset of cliaddr I add also those 3 lines:

cliaddr.sin_family = AF_INET;
cliaddr.sin_addr.s_addr = htonl(INADDR_ANY);
cliaddr.sin_port = htons(SERV_PORT);

I get a random behavior. Sometimes I get 0.0.0.0, sometimes 127.0.0.1 and sometimes the correct address (192.168.1.251).

Community
  • 1
  • 1
Robb1
  • 4,587
  • 6
  • 31
  • 60
  • Minor: `(*sndaddr).sin_addr` = `sndaddr->sin_addr` – Eugene Sh. Jan 22 '18 at 18:29
  • 2
    Just because you have functions that accept pointer arguments does not mean you need to use dynamic memory allocation. Often you don't. Instead, use the address-of operator (`&`) when you can to create a pointer to an ordinary object of the appropriate type. Or sometimes it's appropriate to use an array. – John Bollinger Jan 22 '18 at 18:34
  • Thank you both @EugeneSh. and @JohnBollinger for your comments. I'll try to follow you hints from now on. Do you have any idea on why I get `0.0.0.0`? It really puzzles me. :( – Robb1 Jan 22 '18 at 18:38

2 Answers2

4

you are passing the wrong length to recvfrom

  socklen_t *plen = malloc(sizeof(struct sockaddr *));
    if ((recvfrom(sockfd, buff, sizeof(buff), 0, (struct sockaddr *) sndaddr, plen))

Why you malloc is a mystery, but you need

socklen_t *plen = malloc(sizeof(socklen_t));
*plen = sizeof(struct sockaddr_in );

much simpler is

      socklen_t plen = sizeof(struct sockaddr_in);
   if ((recvfrom(sockfd, buff, sizeof(buff), 0, (struct sockaddr *) sndaddr, &plen))
pm100
  • 48,078
  • 23
  • 82
  • 145
  • I'm surprised this issue hasn't received more attention. I've been stuck on this for days and after reading this, I realized that all 3 of the sources I used as examples for `recvfrom` were wrong! None of them initialized the actual length of a `sockaddr_in`! The sneaky thing is, not doing this will still work about 50% of the time and the payload arrives seemingly always intact. – Feliks Montez Mar 27 '20 at 21:07
-3

It should work if you fill plen with valid sizeof data.

*plen = sizeof(struct sockaddr_in);

You can put that right before the recvfrom call.

Snohdo
  • 156
  • 5
  • You are right, thanks for the answer: I fixed it, but the wrong ip issue is not solved of course. – Robb1 Jan 22 '18 at 18:53