need some help with a printf function

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
Pyrofan1
Member
Member
Posts: 234
Joined: Sun Apr 29, 2007 1:13 am

Post 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++;
			}
		}
	}
}
jnc100
Member
Member
Posts: 775
Joined: Mon Apr 09, 2007 12:10 pm
Location: London, UK
Contact:

Post 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.
Pyrofan1
Member
Member
Posts: 234
Joined: Sun Apr 29, 2007 1:13 am

Post 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)
User avatar
mathematician
Member
Member
Posts: 437
Joined: Fri Dec 15, 2006 5:26 pm
Location: Church Stretton Uk

Post 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;
Pyrofan1
Member
Member
Posts: 234
Joined: Sun Apr 29, 2007 1:13 am

Post 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)
urxae
Member
Member
Posts: 149
Joined: Sun Jul 30, 2006 8:16 am
Location: The Netherlands

Post 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.
User avatar
B.E
Member
Member
Posts: 275
Joined: Sat Oct 21, 2006 5:29 pm
Location: Brisbane Australia
Contact:

Post 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';
      } 
Image
Microsoft: "let everyone run after us. We'll just INNOV~1"
Pyrofan1
Member
Member
Posts: 234
Joined: Sun Apr 29, 2007 1:13 am

Post 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)
User avatar
B.E
Member
Member
Posts: 275
Joined: Sat Oct 21, 2006 5:29 pm
Location: Brisbane Australia
Contact:

Post 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).
Image
Microsoft: "let everyone run after us. We'll just INNOV~1"
Post Reply