Thread: Help with my function (Heh)

Code:

ident[nicklen] = '\0'; // This is your buffer overflow.

nicklen is equal to the size of your array based on the statement before this. And as you might know, the size of an array is not an index in the array as it goes from 0 to (size-1)

Now this would matter ofcourse if you were allocating memory into these pointers, but you aren't. You have to use malloc(). It's in the C programming tutorials.

Merely as an aside -- the other items mentioned are the root of your problem -- but this just strikes me as curious:

Code:

int identlen = (size_t) strlen(ident);

Since strlen returns a size_t, why would you explicitly cast the return value from a size_t to a size_t, and then assign it to an int?

It seems as though you're afraid of using a size_t value due to unfamiliarity, perhaps, but then using a cast with something you are unfamiliar with seems a dangerous proposition.


And are you using C99 or C++ for the declarations that follow statements?

[edit]By the way, I might go with something like this to start.

Code:

#include <stdio.h>

int main(void)
{
   const char source[] = "[email protected]";
   char host[128], nick[128], ident[128];
   if ( sscanf(source, "%127[^!]!%127[^@]@%127[^\n]", nick, ident, host) == 3 )
   {
      printf(" nick: %s\n", nick);
      printf("ident: %s\n", ident);
      printf(" host: %s\n", host);
   }
   return 0;
}

/* my output
 nick: nick
ident: ident
 host: a.random.long.stupid.host.192.168.0.1.com
*/

Thank you all for your quick reply and pointers!

I've updated the code and it works very well.. almost.

Code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main(void)
{
	char	*source		= "[email protected]";

	int		maxnicklen	= 32;
	int		maxidlen	= 32;

	char	*host		= malloc(sizeof source);
	char	*nick		= malloc(maxnicklen); 
	char	*ident		= malloc(maxidlen);

	int		i			= 0;

	host = source;

	/* nick */
	while ((host[i] != '!') && (i < maxnicklen))  
	{
		nick[i] = host[i];
		i++;
	}

	int nicklen = strlen(nick);
	ident[nicklen] = '\0';
	host += nicklen +1;

	/* ident */
	i = 0;
	while ((host[i] != '@') && (i < maxidlen)) 
	{
		ident[i] = host[i];
		i++;
	}

	int identlen = strlen(ident);
	ident[identlen] = '\0'; 
	host += identlen +1;

	printf("%s (%s @ %s)\n", nick, ident, host);

	/*
	free(host);
	free(nick);
	free(ident);
	*/

	return 0;

}

However, when I add "free (host);" (and nick, ident) it segfaults! Any ideas?

Dave_Sinkula: That sscanf is pretty cool, thanks.

(Btw, my indenting isn't as bad as it might look on the forum :P)

Code:

int nicklen = strlen(nick);
nick[nicklen] = '\0';

What do you think the strlen of nick is if nick isn't null terminated? I made a few changes to your code so it works properly. I'm also not sure why you're getting segmentation faults, but I'd guess it's due to the fact that you're out of bounds with your null characters.

Code:

#include <stdio.h>
#include <string.h>

const int maxnicklen	= 32;
const int maxidlen	= 32;

int main(void)
{
   char *source = "[email protected]";

   char *host = malloc(strlen(source));
   char *nick = malloc(maxnicklen); 
   char *ident = malloc(maxidlen);
	
   int i = 0;

   host = source;

   while (*host != '!' && i < maxnicklen) {
    nick[i] = *host;
    i++;
    host++;
   }
	
   nick[i] = '\0';
   host++;
   
   i = 0;
   while (*host != '@' && i < maxidlen) {
     ident[i] = *host;
     i++;
     host++;
   }
	
   ident[i] = '\0'; 
   host++;
	
   printf("Nick:  %s \n"
          "Ident: %s \n"
          "Host:  %s \n", 
          nick, ident, host);
	
   free(host);
   free(nick);
   free(ident);
	
   return 0;
}

Take a look at this and see what it's doing differently. I kept it along the lines of what you were doing and made as few changes as possible. In reality, you'd probably go with something like Dave posted.

@ SlyMaelstrom
I count at least 5 bugs in your code.

> char *host = malloc(strlen(source));
1. Didn't include stdlib.h
2. Didn't account for the \0

> host = source;
This doesn't copy the string, it does however
3. introduce a memory leak.
4. crash when you try and free it later on, since you're freeing what you didn't malloc.

> while (*host != '!' && i < maxnicklen)
5. If you exit on the 2nd condition, the \0 character which follows is out of bounds.
This is repeated in the next loop as well (so perhaps 6 altogether?)

Sly's code is broken.

Code:

while (*host != '@' && i < maxidlen) {
     ident[i] = *host;
     i++;
     host++;
}
	
ident[i] = '\0';

What happens when i is (in this case) 31 at the start of the loop, and you still haven't found a @? BadThings(TM), that's what.


Quzah.

Originally Posted by quzah

What happens when i is (in this case) 31 at the start of the loop, and you still haven't found a @? BadThings(TM), that's what.

Mmmm good point. Threw it together without seeing that. I'll credit myself for that discovery and put your name in the special thanks list.