|
Home > Archive > Unix Programming > June 2006 > recv() problem
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
|
|
|
| I have this thing which is just making me pulling my hairs out. I don't know
what i should do. I'd rather not use select, because i don't have much
experience with that and would make the code rather difficult to read for me.
The problem is, how do i know if there is still data to read on the sending
side? blocking sockets apparently don't work, because once you do a recv(),
it blocks. This is also true for MSG_PEEK flag. So i started to use
nonblocking sockets. I tried different pieces of code, but i don't seem to
get it right. It either hangs, but sometimes does get data.
It uses a crude timer to get remaining data on the server end.
This function is the culprit:
int transmit_data(int fd,int mode,char *send_buffer)
{
/* This function pushes both receive and send buffers to
the timer value set by t_val. Returns 0 upon success and
-1 upon failure or partial failure */
char receive_buffer[100000];
int r=0;
int t_val=0;
/* Set socket tot NONBLOCKING */
if(fcntl(fd,F_SETFL,O_NONBLOCK)==-1) {
perror("fcntl");
exit(1);}
if(!mode) { /* 0=receive data */
do {
bzero(receive_buffer,sizeof(receive_buff
er));
r=recv(fd,receive_buffer,sizeof(receive_
buffer),0);
if(r==-1) { /* Indicates EAGAIN, we try again until t_val reaches 1000 */
t_val++;
continue;}
if(r==0) return 0; /* Connection closed */
if(t_val==1000) return -1; /* EAGAIN max reached, return -1 */
if(r!=-1) { /* We got something */
receive_buffer[r]='\0';
printf("%s\n",receive_buffer);
} while(r!=0);}
return 0;
} else { /* 1=sent data */
do {
r=+send(fd,send_buffer,sizeof(*send_buff
er),0);
} while(r!=sizeof(*send_buffer));
}
if(r==sizeof(*send_buffer))
return 0;
else
return -1;
If anyone can tell me what is wrong with this, please, i would VERY much
appreciate it.
--
| |
| Rainer Temme 2006-06-27, 1:24 pm |
| alef wrote:
> /* Set socket tot NONBLOCKING */
> if(fcntl(fd,F_SETFL,O_NONBLOCK)==-1) {
> perror("fcntl");
> exit(1);}
Setting the socket non-blocking is a good idea in your case.
However, if this is done once (after the creation of the socket)
it is not required to do it again, unless you switch the socket back to
blocking-mode somewhere
> do {
> bzero(receive_buffer,sizeof(receive_buff
er));
> r=recv(fd,receive_buffer,sizeof(receive_
buffer),0);
> if(r==-1) { /* Indicates EAGAIN, we try again until t_val reaches 1000 */
> t_val++;
> continue;}
> if(r==0) return 0; /* Connection closed */
> if(t_val==1000) return -1; /* EAGAIN max reached, return -1 */
> if(r!=-1) { /* We got something */
> receive_buffer[r]='\0';
> printf("%s\n",receive_buffer);
> } while(r!=0);}
In the worst case, you do a busy loop over a socket that has no
data for you in the moment ...
Why don't you use poll() or select() to find out if there
is anything to read?
Rainer
| |
| Rainer Temme 2006-06-27, 1:24 pm |
| alef wrote:
> do {
> bzero(receive_buffer,sizeof(receive_buff
er));
> r=recv(fd,receive_buffer,sizeof(receive_
buffer),0);
> if(r==-1) { /* Indicates EAGAIN, we try again until t_val reaches 1000 */
> t_val++;
> continue;}
> if(r==0) return 0; /* Connection closed */
> if(t_val==1000) return -1; /* EAGAIN max reached, return -1 */
> if(r!=-1) { /* We got something */
> receive_buffer[r]='\0';
> printf("%s\n",receive_buffer);
> } while(r!=0);}
Hm let me reorder this ...
do {
bzero(receive_buffer,sizeof(receive_buff
er));
r=recv(fd,receive_buffer,sizeof(receive_
buffer),0);
if(r==-1)
{
t_val++;
continue;
}
if(r==0) return 0;
if (t_val==1000) return -1;
if (r!=-1)
{
receive_buffer[r]='\0';
printf("%s\n",receive_buffer);
}
} while(r!=0);
Now it's very easy to see, that the statement
"if (t_val==1000) return -1;"
is never reached, because the continue will go
back to the beginning of the do-loop immediately.
See my other mail regarding the busy-loop being replaced
with select() or poll()
Rainer
| |
| Alex Fraser 2006-06-27, 1:24 pm |
| "alef" <alef@SPAMxs4all.nl> wrote in message
news:0001HW.C0C6DA9E06137CE0F0407530@news.xs4all.nl...
>I have this thing which is just making me pulling my hairs out. I don't
>know
> what i should do. I'd rather not use select, because i don't have much
> experience with that and would make the code rather difficult to read for
> me.
>
> The problem is, how do i know if there is still data to read on the
> sending
> side?
Usually, the protocol allows you to determine this, else it has a design
fault.
It is not clear from your description or code exactly what your aims are. It
would almost certainly be easier to help you if you described the situation
more fully.
As for your code, it is counter-intuitive to have a function called
"transmit_data" which, depending on its arguments, may attempt to receive
data rather than transmit it. I suggest splitting it into two functions.
[snip]
> int transmit_data(int fd,int mode,char *send_buffer)
> {
> /* This function pushes both receive and send buffers to
> the timer value set by t_val. Returns 0 upon success and
> -1 upon failure or partial failure */
> char receive_buffer[100000];
This buffer is unnecessarily large. A few kilobytes (say, four or eight)
usually works well.
> int r=0;
> int t_val=0;
>
> /* Set socket tot NONBLOCKING */
> if(fcntl(fd,F_SETFL,O_NONBLOCK)==-1) {
> perror("fcntl");
> exit(1);}
As Rainer noted, this only needs to be done once, unless the descriptor is
set back to blocking elsewhere. To set non-blocking mode with fcntl(),
*modify* the flags by first retrieving the current ones. Something like:
int flags = fcntl(fd, F_GETFL);
if (flags == -1) error();
if (fcntl(fd, F_SETFL, flags | O_NONBLOCK)) error();
> if(!mode) { /* 0=receive data */
> do {
> bzero(receive_buffer,sizeof(receive_buff
er));
memset() is preferred over bzero(). In this case, neither is necessary
anyway.
> r=recv(fd,receive_buffer,sizeof(receive_
buffer),0);
> if(r==-1) { /* Indicates EAGAIN, we try again until t_val reaches 1000 */
There are other possible errors. Test errno.
> t_val++;
> continue;}
> if(r==0) return 0; /* Connection closed */
> if(t_val==1000) return -1; /* EAGAIN max reached, return -1 */
This test is only reached when recv() returns something other than 0 and -1.
(If it returns -1, control jumps from the "continue" above to just before
the end of the loop.)
> if(r!=-1) { /* We got something */
> receive_buffer[r]='\0';
This will write beyond the end of receive_buffer if recv() completely fills
it. Since you zeroed the whole buffer anyway, if recv() does /not/
completely fill it, receive_buffer[r] will be 0 anyway.
I suggest getting rid of the bzero() call above, and modifying the recv()
call to pass sizeof receive_buffer - 1 as the size.
> printf("%s\n",receive_buffer);
> } while(r!=0);}
>
> return 0;
> } else { /* 1=sent data */
> do {
> r=+send(fd,send_buffer,sizeof(*send_buff
er),0);
Did you mean "+=" above? If so, don't do that. Store the return value so you
can test for errors.
"sizeof *send_buffer" is guaranteed to be 1 (sizeof (char)). You need to
pass the size to your function separately.
> } while(r!=sizeof(*send_buffer));
> }
> if(r==sizeof(*send_buffer))
> return 0;
> else
> return -1;
The precise semantics you intend for sending are unclear, but I doubt they
match the code you have written.
Alex
| |
| davids@webmaster.com 2006-06-28, 1:20 am |
|
alef wrote:
> The problem is, how do i know if there is still data to read on the sending
> side?
> blocking sockets apparently don't work, because once you do a recv(),
> it blocks. This is also true for MSG_PEEK flag. So i started to use
> nonblocking sockets. I tried different pieces of code, but i don't seem to
> get it right. It either hangs, but sometimes does get data.
> If anyone can tell me what is wrong with this, please, i would VERY much
> appreciate it.
Your question makes no sense. Data on the sending side is not waiting
to read, it's waiting to be sent.
What protocol are you trying to implement on top of TCP? (The correct
way to get the information I think you are looking for is different for
each protocol that uses TCP.)
DS
| |
| Alef Veld 2006-06-28, 7:30 am |
| Hi,
Thank you for your help. I'm trying to write a simple channelbot for IRC. I
will look into the RFC for specs regarding if the protocol sends the amount
of data it sends upfront.
In the meantime, i still haven't exactly figured out what the problem is.
The continue statement i did get, and i fixed that hopefully. Although i
Don't want to ask anyone here for doing my 'homework', a piece of
Example code on how to handle this situation best would be very appreciated.
I already tried to look through source code of other irc clients, like
eggdrop or bitchx, but they are just way to complicated for a simple guy
like me :-)
The transmit_data was just a choice of efficiency. But maybe i will later
split it up into 2 functions, although i must say i don't see the added
benefit yet.
On 6/27/06 5:54 PM, in article
S7GdndubV5QryTzZnZ2dnUVZ8tKdnZ2d@eclipse
.net.uk, "Alex Fraser"
<me@privacy.net> wrote:
> "alef" <alef@SPAMxs4all.nl> wrote in message
> news:0001HW.C0C6DA9E06137CE0F0407530@news.xs4all.nl...
>
> Usually, the protocol allows you to determine this, else it has a design
> fault.
>
> It is not clear from your description or code exactly what your aims are. It
> would almost certainly be easier to help you if you described the situation
> more fully.
>
> As for your code, it is counter-intuitive to have a function called
> "transmit_data" which, depending on its arguments, may attempt to receive
> data rather than transmit it. I suggest splitting it into two functions.
>
> [snip]
>
> This buffer is unnecessarily large. A few kilobytes (say, four or eight)
> usually works well.
>
>
> As Rainer noted, this only needs to be done once, unless the descriptor is
> set back to blocking elsewhere. To set non-blocking mode with fcntl(),
> *modify* the flags by first retrieving the current ones. Something like:
>
> int flags = fcntl(fd, F_GETFL);
> if (flags == -1) error();
> if (fcntl(fd, F_SETFL, flags | O_NONBLOCK)) error();
>
>
> memset() is preferred over bzero(). In this case, neither is necessary
> anyway.
>
>
> There are other possible errors. Test errno.
>
>
> This test is only reached when recv() returns something other than 0 and -1.
> (If it returns -1, control jumps from the "continue" above to just before
> the end of the loop.)
>
>
> This will write beyond the end of receive_buffer if recv() completely fills
> it. Since you zeroed the whole buffer anyway, if recv() does /not/
> completely fill it, receive_buffer[r] will be 0 anyway.
>
> I suggest getting rid of the bzero() call above, and modifying the recv()
> call to pass sizeof receive_buffer - 1 as the size.
>
>
> Did you mean "+=" above? If so, don't do that. Store the return value so you
> can test for errors.
>
> "sizeof *send_buffer" is guaranteed to be 1 (sizeof (char)). You need to
> pass the size to your function separately.
>
>
> The precise semantics you intend for sending are unclear, but I doubt they
> match the code you have written.
>
> Alex
>
>
| |
| Alex Fraser 2006-06-28, 1:27 pm |
| "Alef Veld" <alef@xs4all.nl> wrote in message
news:C0C81E59.B72%alef@xs4all.nl...
> Thank you for your help. I'm trying to write a simple channelbot for IRC.
An IRC client should constantly pay attention to incoming data. That
basically means you need to use multiple threads, non-blocking I/O plus
select()/poll(), or some other similar mechanism.
Non-blocking I/O plus select()/poll(), using a single thread, fits well with
an event-driven design, which (to me at least) seems the obvious design for
an IRC 'bot.
> I will look into the RFC for specs regarding if the protocol sends the
> amount
> of data it sends upfront.
It is a line-based protocol. That is, no length is sent "upfront" but each
message is terminated by a CR,LF pair ("\r\n").
However, an IRC server may send data at any time (hence my first point).
This is in contrast to many (perhaps most) other protocols.
[snip]
> The transmit_data was just a choice of efficiency. But maybe i will later
> split it up into 2 functions, although i must say i don't see the added
> benefit yet.
An added benefit is that your code is easier to read and understand - by
someone else, or by yourself six months from now. The value of that cannot
be underestimated.
Alex
|
|
|
|
|