Socket only receiving once

[fellixombc]

Everything else works fine, but for some reason, it only receives once. (note, the recv function starts at line 49) function is at

code:

/* 
 * File:   Bot.cpp
 * Author: fellixombc
 * 
 * Created on May 20, 2010, 6:25 PM
 */

#include "bot.h"

 /** Constructs the Bot
  * @param set the username
  * @param set the default channel
  */
Bot::Bot(string param, string param1) {

    botname = param;
    channel = param1;

}

 /** Connects to desired server and port
  * @param set the server
  * @param set the server's port
  */
void Bot::Connect(char* server, int port) {

  host = gethostbyname(server);

  client = socket(AF_INET, SOCK_STREAM, 0);
  address.sin_family = AF_INET;
  address.sin_port = htons(port);
  address.sin_addr.s_addr = *((unsigned long*)host->h_addr);
  
  if(connect(client, (sockaddr*)(&address), sizeof(address)) == -1) {
            cout << "Error..." << endl;
            Disconnect();
  }
  
  cout << "Connected!" << endl;

  Send("NICK " + botname + "\r\n");
  Send("User " + botname + " " + botname + "1 " + botname + "2 :C++ Irc Bot \r\n");

}

 /** Constructs the Bot
  * @param points the recv'd text to the input
  */
void Bot::Recv(string& buffer) {

    while(size == 0) {
        ioctl(client, FIONREAD, &size);
    }

    char* buf = new char[size+1];
    
    bytes = recv(client, buf, size, 0);

    cout << bytes << endl;

    buffer = buf;
    delete[] buf;
/*
    if(buffer.find("/MOTD") != -1) {
        Send("JOIN " + channel + " \r\n");
    }*/
}

 /** Sends a message to the server
  */
void Bot::Send(string msg) {
    send(client, msg.c_str(), sizeof(msg), 0);
    cout << msg;
}

 /** Closes the socket
  */
void Bot::Disconnect() {
    close(client);
    cout << "Disconnected." << endl;
}

How it is being used:

/*
 * File:   main.cpp
 * Author: Fellixombc
 * Purpose: Socket example
 *
 * Created on May 18, 2010, 8:40 PM
 */

#include <iostream>

#include "bot.h"

using namespace std;

int main() {

    string buffer;

    Bot bot("Bob", "#bobbot");
    bot.Connect("godirc.com", 6667);

    while(1) {
        bot.Recv(buffer);
        cout << buffer;
    }

    return 0;
}

Any idea?

[Aranarth]

Problems I'm seeing:
1. I don't see size being initialized anywhere, so even the first call to Recv could crash the program due to trying to allocate ridiculous amounts of memory.
2. You are not checking the return value of recv to see whether the connection has been terminated. Perhaps this is what actually happens.
3. When assigning buf to buffer, buf is treated as a nullterminated char string. You're not adding the null termination byte anywhere, so this could have bad effects. Add a buf[bytes]=0;
4. Perhaps the ioctl call causes problems. You're not checking the return value either. It would probably be better to just get rid of the call and have a fixed buffer. Especially if ioctl does not block - in that case your current way would cause 100% CPU load.
const int size=65536;
char buf;

[fellixombc]

Problems I'm seeing:
1. I don't see size being initialized anywhere, so even the first call to Recv could crash the program due to trying to allocate ridiculous amounts of memory.
2. You are not checking the return value of recv to see whether the connection has been terminated. Perhaps this is what actually happens.
3. When assigning buf to buffer, buf is treated as a nullterminated char string. You're not adding the null termination byte anywhere, so this could have bad effects. Add a buf[bytes]=0;
4. Perhaps the ioctl call causes problems. You're not checking the return value either. It would probably be better to just get rid of the call and have a fixed buffer. Especially if ioctl does not block - in that case your current way would cause 100% CPU load.
const int size=65536;
char buf;

size is being defined here: ioctl(client, FIONREAD, &size);

ioctl writes the size to my size variable. And, here is my new code: (still with ioctl)

void Bot::Recv(string& buffer) {

    while(size == 0) {
        ioctl(client, FIONREAD, &size);
    }

    char* buf = new char[size+1];
    
    bytes = recv(client, buf, size, 0);

    buf[size] = '\0';

    buffer = buf;

    if(bytes == 0) {
        cout << "Disconnected" << endl;
    }

    if(buffer.find("/MOTD") != -1) {
        Send("JOIN " + channel + " \r\n");
    }

    delete[] buf;
}

also, ioctl is important, it gets the size of the message being sent to us, while using a predetermined size can cause problems; it will recv the same line more then once.

Also, same thing happens when I removed ioctl:

void Bot::Recv(string& buffer) {

    const int size = 512;
    char* buf = new char[size+1];

    cout << size << endl;
    
    bytes = recv(client, buf, size, 0);

    buf[size] = '\0';

    buffer = buf;

    if(bytes == 0) {
        cout << "Disconnected" << endl;
    }

    if(buffer.find("/MOTD") != -1) {
        Send("JOIN " + channel + " \r\n");
    }

    delete[] buf;
}

[Aranarth]

size is being defined here: ioctl(client, FIONREAD, &size);

It is not! It's just used here. I assume it is a class member or a global variable (which should be local). If it is a class member, it will be uninitialized when calling Recv for the first time. It could have a huge value such as 2 billion... this is obviously non-zero, so ioctl is never called. Please note that even if size happens to be 0 initially, ioctl will be called at most once, because size is guaranteed to be non-zero after the first call to Recv.

also, ioctl is important, it gets the size of the message being sent to us, while using a predetermined size can cause problems; it will recv the same line more then once.

No, it won't, because recv returns how many bytes were actually received. If the buffer is too small to hold all the data, you'll get the rest (and only the rest) with the next call to recv. Note that recv can return a value below zero - this usually indicates that the connection has been closed due to an error. It's important to handle this case.

One more thing, don't use new[] and delete[] to allocate a small temporary buffer. These are much more costly than allocating the buffer on the stack. Actually, allocating it on the stack is free (literally), because it requires a single CPU instruction to adjust the stack pointer. Since that usually has to be done anyway in a function, it will be merged with the existing instruction. So the whole thing doesn't cost you one single CPU instruction.

If you still have problems, you should post the bot.h, so I'm able to verify the problem myself.

[fellixombc]

gah, still, the program only recvs twice, and with octl in effect, i get disconnected after my first bytes i recv.

new code for Recv

void Bot::Recv(string& buffer) {

    int size = 1024;
   // ioctl(client, FIONREAD, &size);

    char buf[size];
    cout << size << endl;
    
    bytes = recv(client, buf, size, 0);
    cout << bytes << endl;

    buf[size] = '\0';
    buffer = buf;

    if(bytes == 0) {
        cout << "Disconnected" << endl;
    }

    if(buffer.find("/MOTD") != -1) {
        Send("JOIN " + channel + " \r\n");
    }

}

Anyways, as requested. bot.h:

/* 
 * File:   bot.h
 * Author: fellixombc
 *
 * Created on May 20, 2010, 6:25 PM
 */

#ifndef BOT_H
#define	BOT_H

/* Libraries */
#include <iostream>
#include <string>
#include <vector>

#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netdb.h>
#include <arpa/inet.h>

/* Namespaces */
using namespace std;

class Bot {
public:
    Bot(string, string);
    void Connect(char*, int);
    void Recv(string&);
    void Send(string);
    void Disconnect();
private:
    int client;
    int bytes;

    string botname;
    string channel;

    struct sockaddr_in address;
    struct hostent *host;
};

#endif	/* BOT_H */

[Aranarth]

The remaining problem is in Send. You're trying to use sizeof to retrieve the length of the string, but sizeof gives you the size of the string structure (depends on the implementation, but usually that's just 4 bytes).
What you need is msg.length().

[fellixombc]

The remaining problem is in Send. You're trying to use sizeof to retrieve the length of the string, but sizeof gives you the size of the string structure (depends on the implementation, but usually that's just 4 bytes).
What you need is msg.length().

Ahh simple mistake, thank you very much!