5

I am new to socket programming... I tried this server side program

#define BUFLEN 512
#define MYPORT 3456
void errorp(char* msg)
{
        perror(msg);
        exit(1);
}

int main()
{
        struct sockaddr_in server, client;
        int sock;
        int slen = sizeof(server);
        int clen = sizeof(client);
        char *recvbuf, senbuf[BUFLEN] = {'h','e','l','l','o'};

        if((sock = socket(AF_INET, SOCK_DGRAM, 0) == -1))
                errorp("Socket creation failed");


        printf("To the client: %s, %s", senbuf, " World");
        bzero(&server, sizeof(server));

        server.sin_family = AF_INET;
        server.sin_port = MYPORT;
        server.sin_addr.s_addr = inet_addr("127.0.0.1");
        if(bind(sock, (struct sockaddr*)&server, slen)==-1)
                errorp("Socket Bind Failed");
        if(recvfrom(sock, recvbuf, sizeof(recvbuf), 0, (struct sockaddr*) &client, &clen) == -1)
                errorp("recv from error");
        printf("From the client: %s", recvbuf);

        if(sendto(sock, senbuf, sizeof(senbuf), 0, (struct sockaddr*) &client, sizeof(client)) == -1)
                errorp("Error in sending");

        printf("To the client: %s", senbuf);
        close(sock);
        return 0;
}

There are no compilation errors but the output is

Socket Bind Failed: Socket operation on non-socket
To the client: hello,  World

Please help me figure out where the mistake is? and help get rid of it

PrathapB
  • 382
  • 1
  • 4
  • 15
  • 2
    Side note: the port number has to be converted to network byte order: `server.sin_port = htons(MYPORT);` . – Martin R Aug 31 '14 at 10:19
  • 2
    "*There are no compilation errors ...*" but there are warnings at least if you'd compiled using the options `-Wal -Wextra -pedantic`. – alk Aug 31 '14 at 10:44
  • `-Wal` should have read `-Wall`. – alk Aug 31 '14 at 11:12

2 Answers2

18

The error message says it all: The socket isn't a (valid) socket.

This should make you look at the code creating the socket:

if((sock = socket(AF_INET, SOCK_DGRAM, 0) == -1))

The code above 1st compares the result of the call to socket() to -1 and then assigns the result of the comparison to sock. So it's either 0 or 1. And the result of the call to socket() is lost.

The code shall look like this:

if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) == -1)

as == binds tighter then =.


BTW, having used a Yoda-Conditition would have avoided such kind of "typo":

if (-1 == (sock = socket(AF_INET, SOCK_DGRAM, 0)))

Also at least clen shall be of type socklen_t as its address is passed, to have a value written into it, which will fail miserably if the size of the expected socklen_t would be different from an int (which the code shown passes).

alk
  • 69,737
  • 10
  • 105
  • 255
  • 2
    Well spotted. And once that's fixed, the next fatal error is UB because recvbuf is used uninitialized. – John Zwinck Aug 31 '14 at 10:18
  • @JohnZwinck But i din't get any such errors.. cud you explain wat UB is? – PrathapB Aug 31 '14 at 10:46
  • @PrathapB: undefined behavior. Your recvbuf is garbage. – John Zwinck Aug 31 '14 at 11:06
  • @JohnZwinck: "*Your recvbuf is garbage.*" it isn't at all! ;-) – alk Aug 31 '14 at 11:09
  • @PrathapB: You might like to read here: http://stackoverflow.com/q/2397984/694576 – alk Aug 31 '14 at 11:11
  • It's an uninitialized pointer, ie. garbage, as described by @JohnZwinck. The there's 'sizeof(recvbuf)' - gonna be 4 or 8. Even if you malloc recvbuf, 'printf("From the client: %s", recvbuf)' will likely blow up if the datagram is not null-terminated. – Martin James Sep 01 '14 at 03:04
10
if((sock = socket(AF_INET, SOCK_DGRAM, 0) == -1))
//         \__________________________________/

You have your brackets in the wrong place. It's setting sock to a true/false value because == is "more binding" than =. It should instead be:

if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
//  \_____________________________________/

which sets sock to the return value from socket() and then compares that to -1.

You also have no backing storage for recvbuf which means your recvfrom(), once it starts working, will almost certainly do something bad.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953