And now you learn the crucial difference between "compiles" and "does what I thought it did"..
Did you do #include <string.h>, for starters?
Taking a closer look at the code now...
1 Like
Yes, I have both string.h and strings.h - they were both included before I modified the file and didn't want to take anything out just yet.
I'm not positive the segfaults are related to the code change - I think they may have been happening before I modified anything. Don't want to waste your time, just in case.
// fill the area of memory taken by serv_addr structure with zeros
bzero((char *) &serv_addr, sizeof(serv_addr));
// set the sin_family member of serv_addr to AF_INET (IP Address Family)
serv_addr.sin_family = AF_INET;
// copy the address returned by gethostbyname() into the s_addr member of serv_addr
// s_addr contains the integer representation of the IP address
bcopy((char *)server->h_addr, (char *)&serv_addr.sin_addr.s_addr, server->h_length);
// set the sin_port member of serv_addr to the port number
// htons converts a u_short from host to IP network byte order (which is big-endian).
serv_addr.sin_port = htons(portno);
1 Like
When I pasted this into a C function it complained about two undefined variables -- fds, and timeout. That's it.
So, I don't think you're actually storing the socket you opened into the class anywhere.
Also, you can remove buffer[]. You're not using it so its useless.
Beyond that, your code compiles and works for me.
1 Like
Thank you. I appreciate that.
Hmm... ok. Do you have any suggestions on what I should do? I just heard they're considering getting an IBM engineer involved to help out with this - I'd like to avoid that if at all possible.
So you open the socket, but don't keep it. And when later code tries to use it, it uses an empty, default, or garbage value instead of the socket you opened... which wouldn't cause a crash directly, but if they didn't catch certain error conditions could cause that.
1 Like
Ok - here is my connection class. Maybe that's why you're not seeing those members? I don't know.
class connection
{
private:
SOCKET the_socket;
char * in_buffer;
char * out_buffer;
unsigned int in_index;
unsigned int out_index;
unsigned int in_buffer_total;
unsigned int out_buffer_total;
unsigned int last_winsock_error;
TASK_HANDLE_TYPE owner_task;
fd_set fds;
struct timeval timeout;
public:
connection (void);
~connection (void);
int get_connected (char * hostname, char * service);
SOCKET get_socket(void)
{
return (the_socket);
}
TASK_HANDLE_TYPE get_owner_task(void)
{
return (owner_task);
}
int get_buffer(int wait);
int close (void);
int getchar (int wait, char * ch);
int put_data (char * data, unsigned long length);
int put_data_buffered (char * data, unsigned long length);
int put_data_flush (void);
};
I think you're supposed to do the_socket=sockfd; after you open the connection. Otherwise the class will have no idea it's supposed to use the socket you just opened.
I have no idea what's supposed to set up those character buffers. Hopefully it's set up in the constructor?
1 Like
constructor:
connection::connection (void)
{
the_socket = 0;
in_index = 0;
out_index = 0;
in_buffer_total = 0;
out_buffer_total = 0;
in_buffer = 0;
in_buffer = new char[SOCKET_BUFFER_SIZE];
out_buffer = new char[SOCKET_BUFFER_SIZE];
last_winsock_error = 0;
}
When compiling, I get this on a batch job that uses that lib:
ld: 0711-224 WARNING: Duplicate symbol: std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >::_Copy(unsigned long)
Not sure if that's related at all.
ok, good. I don't think that warning's related.
But, you definitely need to set the_socket=sockfd before you return. The class won't happen to just know what socket you opened -- you have to tell it 
What does the second parameter of get_connected do, or should do? Currently it's being ignored.
1 Like
It was being used here:
// --------------------------------------------------
// resolve the service name
//
// If they've specified a number, just use it.
if (gensock_is_a_number (service))
{
char * tail;
our_port = (int) strtol (service, &tail, 10);
if (tail == service)
{
return (ERR_CANT_RESOLVE_SERVICE);
}
else
{
our_port = htons (our_port);
}
}
else
{
// we have a name, we must resolve it.
serventry = getservbyname (service, "tcp");
if (serventry)
our_port = serventry->s_port;
else
return (ERR_CANT_RESOLVE_SERVICE);
}
But I took it out and just replaced it with portno=25; because &tail wasn't declared anywhere (if I remember correctly). Since this will always be used for e-mail, I figured it was safe to assume a hard coded port of 25.
btw, thank you for helping with this. I'd be lost completely.
---------- Post updated at 11:45 AM ---------- Previous update was at 11:32 AM ----------
Oh, also - I've renamed all occurrences of the_socket to sockfd (including the class definition). Should I take out the declaration of sockfd at the beginning of get_connected() ?
Okay, so the second parameter is the port number, or service name, in the form of a character string.
Not all mail servers have the same port, I'm not sure it's safe to ignore it. Never understand why people still mess with strtol for short ints when they've got scanf. This code should do the job:
if(service == NULL)
return(ERR_CANT_RESOLVE_SERVICE);
// We read it in as local order!
if(sscanf(service, "%d", &portno) == 1);
else if(s = getservbyname(service, "tcp"))
portno=htons(s->s_port); // Convert from network to LOCAL order!
else
return(ERR_CANT_RESOLVE_SERVICE);
You need a 'struct servent *s;' up in the group of variables at the top. You'll need <stdio.h>, which I give good odds that you've included already. And you'll need to stop manually setting it to 25 of course. The htons() is needed since s->s_port is already swapped for you, and you do swapping below too, we just want it as a normal int here...
I tested it on service strings like "telnet", and arbitrary ports like "8888".
---------- Post updated at 11:41 AM ---------- Previous update was at 11:00 AM ----------
Gack... I wouldn't have reccomended doing that. Before, the_socket would never have been changed. With the one change I suggested, it would never have been changed unless you knew for a fact that sockfd was valid. Now, it will never change because the local variable's the same name. Get rid of the local definition and it'll change all the time, potentially getting left at invalid values somewhere inbetween.
Just the one line the_socket=sockfd; right at the bottom of the connection function would have done without altering all your connection code.
1 Like
Ok no problem - I can fix that back. Thanks
---------- Post updated at 03:46 PM ---------- Previous update was at 12:42 PM ----------
Ok great. I'm getting a socket connection now!
Now the problem is still this segfault. I return out of my methods to this point:
int prepare_smtp_message(char * MailAddress, char * destination)
{
char out_data[MAXOUTLINE];
char str[1024];
char *ptr;
int len, startLen;
if ( open_smtp_socket() ) // <- this is where it goes into get_connected()
return -1;
if ( get_smtp_line() != 220 ) // <- segfault
{
smtp_error ("SMTP server error");
return ( -1);
}
And at get_smtp_line() != 220, I get a segfault. I'm just posting on the off chance you've seen this code before.
It'd be nice to see the code of get_smtp_line, then.
I'm not sure you need an IBM engineer as much as a C/C++ coder...
Oh yea, duh. I wasn't sure if it was even making into the function or not. When I try to step in, it blows up.
int get_smtp_line( void )
{
char ch = '.';
char in_data [MAXOUTLINE];
char * index;
int retval = 0;
index = in_data;
while (ch != '\n')
{
if ( (retval = gensock_getchar (SMTPSock, 0, &ch) ) )
{
gensock_error ("gensock_getchar", retval);
return -1;
}
else
{
*index = ch;
index++;
}
}
return atoi(in_data);
}
Try putting fprintf(stderr, "debug statements\n"); inside that function so you can tell at what points its still running.
1 Like
Ok so here's what happens.
It goes here:
int get_smtp_line( void )
{
char ch = '.';
char in_data [MAXOUTLINE];
char * index;
int retval = 0;
index = in_data;
cout << "Before while...\n";
while (ch != '\n')
{
if ( (retval = gensock_getchar (SMTPSock, 0, &ch) ) ) // <- see below
{
cout << "In if...\n";
gensock_error ("gensock_getchar", retval);
return -1;
}
else
{
cout << "In else...\n";
cout << "index: " << *index << endl ;
*index = ch;
index++;
}
}
cout << "returning\n";
return atoi(in_data);
}
then to here:
int gensock_getchar (int st, int wait, char * ch)
{
int retval = 0;
connection *conn;
conn = (connection *)st;
if ((retval = conn->cc_getchar(wait, ch))) // <- see below
return (retval);
else
return (0);
}
Then to here:
int
connection::cc_getchar(int wait, char * ch)
{
int retval;
if (in_index >= in_buffer_total)
{
if ((retval = get_buffer(wait))) // <- see below
return (retval);
}
*ch = in_buffer[in_index++];
return (0);
}
Then to here:
int
connection::get_buffer(int wait)
{
int retval;
int bytes_read = 0;
unsigned long ready_to_read = 0;
// Use select to see if data is waiting...
FD_ZERO (&fds); // <-- CRASHES HERE
FD_SET (the_socket, &fds);
// if wait is set, we are polling, return immediately
if (wait)
{
timeout.tv_sec = 0;
}
else
{
timeout.tv_sec = 0;
}
#ifdef HA
if ((retval = select (0, &fds, NULL, NULL, &timeout))
== SOCKET_ERROR)
{
//printf ( "connection::get_buffer() unexpected error from select\n");
}
// if we don't want to wait
if (!retval && wait)
{
return (WAIT_A_BIT);
}
#endif
// we have data waiting...
bytes_read = recv (the_socket,
in_buffer,
SOCKET_BUFFER_SIZE,
0);
// just in case.
if (bytes_read == 0)
{
// connection terminated (semi-) gracefully by the other side
return (ERR_NOT_CONNECTED);
}
if (bytes_read == SOCKET_ERROR)
{
//printf ("connection::get_buffer() unexpected error\n");
}
// reset buffer indices.
in_buffer_total = bytes_read;
in_index = 0;
return (0);
}
At which point it crashes. Still determining if it's the first iteration or not.
---------- Post updated at 05:00 PM ---------- Previous update was at 04:57 PM ----------
edited*
---------- Post updated at 05:02 PM ---------- Previous update was at 05:00 PM ----------
Edited again to show crash spot in last method.
---------- Post updated at 05:05 PM ---------- Previous update was at 05:02 PM ----------
Hey wait a sec... should I be initializing a new connection that many times? Or just one connection somewhere? I bet that has something to do with it.
That is a very bizzare place for it to crash... for it to crash there, just doing FD_ZERO, means the object itself may have ended up an invalid pointer somehow.
Yes, you shouldn't be making a new object every time you write...
1 Like
Ok I've ran into a bit of a snag. I'm trying to do something similar to this:
typedef void * socktag
int gensock_connect (char * hostname,
char * service,
socktag * pst)
{
int retval;
connection * conn = new connection;
if ((retval = conn->get_connected (hostname, service)))
{
gensock_close(0);
*pst = 0;
return (0);
}
*pst = conn;
return (retval);
}
But *pst never gets assigned the address of conn - it stays 0. But it does hit that line.
[edit] third time's the charm. Gah. What is the value of pst? What does it point to?
1 Like