String parsing problems

[Yungblood]

Hi,
To give you a little background on my project, I'm building an IRC/MySQL gateway daemon. The piece I'm working on is receiving text from the irc client, and parsing it correctly.

I am using a select to handle all the socket communication. Once data is put into the input buffer of my struct, I call my getircmsg() function.

Here is the code I'm having issues with:

int getircmsg(struct client *client) {
  /* Strip char 13 out, and check for char 10 */
  if (!gotline(client)) return 0;
  char cmd[64] = "";
  char from[256] = "";
  char dest[64] = "";
  char line[1024] = "";
  int pos=0, x=0;

/*
 * Input can be in one of the following 2 formats:
 * :nick!email@domail.com COMMAND Destination :Message
 * or
 * COMMAND Destination :Message
 */

  if (client->inbuf[0] == ':') 
    while((client->inbuf[pos]!=10)&&(client->inbuf[pos]!=32)) 
      from[x++]=client->inbuf[pos++];
  from[x]=0;
  while(client->inbuf[pos]==32) pos++;
  x=0;
  while((client->inbuf[pos]!=10)&&(client->inbuf[pos]!=32)) 
    cmd[x++]=client->inbuf[pos++];
  cmd[x]=0;
  while(client->inbuf[pos]==32) pos++;
  x=0;
  while((client->inbuf[pos]!=10)&&(client->inbuf[pos]!=32)) 
    dest[x++]=client->inbuf[pos++];
  dest[x]=0;
  while(client->inbuf[pos]==32) pos++;
  x=0;
  while(client->inbuf[pos]!=10) 
    line[x++]=client->inbuf[pos++];
  line[x]=0;
  x=0;
  pos++;
  while(pos<=client->incur) 
    client->inbuf[x++]=client->inbuf[pos++];
  client->incur=x;
  
  logmsg("From: %s", from);
  logmsg("Cmd: %s", cmd);
  logmsg("Dest: %s", dest);
  logmsg("Line: %s", line);
}

It doesn't always fill in cmd. Sometimes the command is put in the dest var. Any ideas why this might be?

If needed, I can tar the whole project, and share it.
Thanks!
-YungBlood

 

[Salem]

> Input can be in one of the following 2 formats:
That this is false perhaps.

Your code is rife with buffer overflow possibilities.
The while loop you use for copying should
a) be a separate function, to make this code a hell of a lot clearer.
b) check for lengths.
c) check for \0.

> if (!gotline(client)) return 0;
But you return 'garbage' on success.

> logmsg("From: %s", from);
How about logging the input string as well?
You know what the bad output looks like, so compare it to the bad input and figure out how your code broke.


--
If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.

 

[Yungblood]

>> Input can be in one of the following 2 formats:
>That this is false perhaps.
It's true, I know this protocol well. And I've tried sending text manually.

>Your code is rife with buffer overflow possibilities.
>The while loop you use for copying should
>a) be a separate function, to make this code a hell of a lot clearer.
Ok, I made a few.
>b) check for lengths.
I am now.
>c) check for \0.
Before it didn't need to, but I changed it for logging the input.

>> if (!gotline(client)) return 0;
>But you return 'garbage' on success.
The code isn't complete, and nothing is checking the return.

>> logmsg("From: %s", from);
>How about logging the input string as well?
>You know what the bad output looks like, so compare it to the bad input and figure out how your code broke.
I am now...

I'm attaching my complete project so far.

Now I'm getting some blank lines in the input buffer, I'm not sending any blank lines. And not everything I'm sending is getting through. However, everything is getting put in the right place when it gets through.

gatewayd.c contains the main loop.
irccmd.c contains the string parsing code.
misclib.c contains all the helper functions.