Page 2 of 2

Posted: Sat Jun 09, 2007 1:17 pm
by Pyrofan1
nope, that didn't help at all
my current code

Code: Select all

void Vsprintf(char *buf,char *fmt,va_list args)
{
	while(1)
	{
		if(*fmt=='\0')
		{
			break;
			buf++;
			*buf='\0';
		}
		else
		{
			if(*fmt=='%')
			{
				fmt++;
				switch(*fmt)
				{
					case 'd':
					strcat(buf,itoa(va_arg(args,int)));
					buf++;
					break;

					case 'c':
					strcat(buf,(char)va_arg(args,int));
					buf++;
					break;

					case 's':
					strcat(buf,va_arg(args,char*));
					buf++;
					break;

					default:
					*buf=*fmt;
					buf++;
					break;
				}
			}
			else
			{
				*buf=*fmt;
				buf++;
				fmt++;
			}
		}
	}
}

Posted: Sat Jun 09, 2007 1:20 pm
by jnc100
I suppose I should have been more specific when I said increment. Add to buf the length of the string you just concatenated, minus the terminating null.

Regards,
John.

Posted: Sat Jun 09, 2007 1:30 pm
by Pyrofan1
That sorta made it better

Code: Select all

void Vsprintf(char *buf,char *fmt,va_list args)
{
	char *temp;
	while(1)
	{
		if(*fmt=='\0')
		{
			break;
			buf++;
			*buf='\0';
		}
		else
		{
			if(*fmt=='%')
			{
				fmt++;
				switch(*fmt)
				{
					case 'd':
					temp=itoa(va_arg(args,int));
					strcat(buf,temp);
					buf+=strlen(temp);
					break;

					case 'c':
					strcat(buf,(char)va_arg(args,int));
					buf++;
					break;

					case 's':
					temp=va_arg(args,char*);
					strcat(buf,temp);
					buf+=strlen(temp);
					break;

					default:
					*buf=*fmt;
					buf++;
					break;
				}
			}
			else
			{
				*buf=*fmt;
				buf++;
				fmt++;
			}
		}
	}
}
output
Hello world
num is (c with tail)(male symbol)5 c is sc
(bunch of weird symbols)Microsoft sucks a(more weird symbols)

Posted: Sat Jun 09, 2007 2:13 pm
by mathematician

Code: Select all

strcat(buf,(char)va_arg(args,int));
You can only use strcat to append another string to Buf; you can't use it to append a character. Use:

*buf++ = c;

Posted: Sat Jun 09, 2007 2:34 pm
by Pyrofan1
current code

Code: Select all

void Vsprintf(char *buf,char *fmt,va_list args)
{
	char *temp;
	while(1)
	{
		if(*fmt=='\0')
		{
			break;
			buf++;
			*buf='\0';
		}
		else
		{
			if(*fmt=='%')
			{
				fmt++;
				switch(*fmt)
				{
					case 'd':
					temp=itoa(va_arg(args,int));
					strcat(buf,temp);
					buf+=strlen(temp);
					break;

					case 'c':
					*buf++=(char)va_arg(args,int);
					break;

					case 's':
					temp=va_arg(args,char*);
					strcat(buf,temp);
					buf+=strlen(temp);
					break;

					default:
					*buf=*fmt;
					buf++;
					break;
				}
				fmt++;
			}
			else
			{
				*buf=*fmt;
				buf++;
				fmt++;
			}
		}
	}
}
current output
[quote]
Hello world
num is (c with tail)(male symbol) 5 c is a
(bunch of random symbols)Microsoft sucks a(more random symbols)

Posted: Sun Jun 10, 2007 2:49 am
by urxae
In the end clause (*fmt == '\0'), first null-terminate, then break. No need to increment buf, it's already at the end.
Use strcpy instead of strcat since buf already points to the end of the current output string, and it may not be null-terminated (You don't null-terminate at the beginning of the function or after single-character outputs (%c, default switch case, regular character)

For extra points, I also handled the special case of a '%' at the end of a format string.

Also, since the first thing you do in your while(1) loop is check a condition and break if it's false, you can just put the reverse condition in the while() and perform extra actions (i.e. null-termination) after the loop. (This is a style issue, not a correctness issue)

End result:

Code: Select all

void Vsprintf(char *buf,char *fmt,va_list args)
{
   char *temp;
   while(*fmt!='\0')
   {
      if(*fmt=='%')
      {
         fmt++;
         switch(*fmt)
         {
            case 'd':
            temp=itoa(va_arg(args,int));
            strcpy(buf,temp);
            buf+=strlen(temp);
            break;

            case 'c':
            *buf++=(char)va_arg(args,int);
            break;

            case 's':
            temp=va_arg(args,char*);
            strcpy(buf,temp);
            buf+=strlen(temp);
            break;

            case '\0':  // End of string after '%'
            continue;   // Let the loop condition pick it up

            default:
            *buf=*fmt;
            buf++;
            break;
         }
         fmt++;
      }
      else
      {
         *buf=*fmt;
         buf++;
         fmt++;
      }
   }
   *buf='\0';
}
Of course, this will still overflow a buffer if the output is too long for the buffer to handle. To fix that, you could pass in a function pointer to a putc[har]-style function (and optionally a void* to be passed as first argument). At least, that's essentially how the [url=whttp://www.digitalmars.com/d/phobos/std_format.html]D standard library[/url] does it, but they get to use delegates so it's a bit cleaner.

Posted: Sun Jun 10, 2007 3:15 am
by B.E
Hello world
num is (c with tail)(male symbol) 5 c is a
(bunch of random symbols)Microsoft sucks a(more random symbols)
This usally means that the null char is missing.

Although I don't know why the 'break' is _before_ the actual insertion of the Null charector in the following

Code: Select all

      if(*fmt=='\0'){
         break;
         buf++;
         *buf='\0';
      } 

Posted: Sun Jun 10, 2007 11:21 am
by Pyrofan1
my current code

Code: Select all

void Vsprintf(char *buf,char *fmt,va_list args)
{
   char *temp;
   while(*fmt!='\0')
   {
      if(*fmt=='%')
      {
         fmt++;
         switch(*fmt)
         {
            case 'd':
            temp=itoa(va_arg(args,int));
            strcpy(buf,temp);
            buf+=strlen(temp);
            break;

            case 'c':
            *buf++=(char)va_arg(args,int);
            break;

            case 's':
            temp=va_arg(args,char*);
            strcpy(buf,temp);
            buf+=strlen(temp);
            break;

            case '\0':
            continue;

            default:
            *buf=*fmt;
            buf++;
            break;
         }
         fmt++;
      }
      else
      {
         *buf=*fmt;
         buf++;
         fmt++;
      }
   }
   *buf='\0';
}
the output
Hello world
num is %d
c is %c
(what looks like the contents of a file)

Posted: Sun Jun 10, 2007 5:38 pm
by B.E
I just tested your Vsprintf function on a Freebsd system (changing the the itoa to a system sprintf call as freebsd doesn't have a itoa) and all succeeded. I would sugest fixing your itoa to first proceduce the _correct_ (try something like 11) string and second to use dyamic memory as the stack will get overwritten.

The other thing could be your 'va_arg's, are they standard (i.e did you copy it from the header files).