Go to Post I think CD is worth about 3 billion in Google stock. Talk about one hell of a fundraiser! - Joe Matt [more]
Home
Go Back   Chief Delphi > Technical > Programming
CD-Media   CD-Spy  
portal register members calendar search Today's Posts Mark Forums Read FAQ rules

 
Closed Thread
 
Thread Tools Rate Thread Display Modes
  #1   Spotlight this post!  
Unread 08-03-2004, 02:32
Jay Lundy Jay Lundy is offline
Programmer/Driver 2001-2004
FRC #0254 (The Cheesy Poofs)
Team Role: Alumni
 
Join Date: Jun 2001
Rookie Year: 2001
Location: Berkeley, CA
Posts: 320
Jay Lundy is a name known to allJay Lundy is a name known to allJay Lundy is a name known to allJay Lundy is a name known to allJay Lundy is a name known to allJay Lundy is a name known to all
Re: printf bug - beware!

Nice catch. The same problem occurs if you do this:

char tempBuf[] = "Some string >=40 characters...";
printf("%s", tempBuf);

The error is another strcpypgm2ram copied into a 40 char buffer without checking the length of the string in program memory.

printf_lib.c line 203 (unmodified version)

Basically anywhere in the printf libraries where strcpypgm2ram is used (which is quite often) there is a possibility for corruption.
  #2   Spotlight this post!  
Unread 08-03-2004, 10:16
deltacoder1020's Avatar
deltacoder1020 deltacoder1020 is offline
Computer Guy
AKA: Dav
#1020 (The Indiana Prank Monkeys)
Team Role: Programmer
 
Join Date: Jan 2004
Location: Muncie, Indiana
Posts: 340
deltacoder1020 has a spectacular aura aboutdeltacoder1020 has a spectacular aura about
Send a message via AIM to deltacoder1020
Re: printf bug - beware!

tsk tsk IFI... this is something you think they would put into the docs... (and if I missed it, oh well, i was speedreading :S)
__________________
Team 1020, the Indiana Prank Monkeys (www.team1020.org)
  #3   Spotlight this post!  
Unread 08-03-2004, 11:34
Mark McLeod's Avatar
Mark McLeod Mark McLeod is offline
Just Itinerant
AKA: Hey dad...Father...MARK
FRC #0358 (Robotic Eagles)
Team Role: Engineer
 
Join Date: Mar 2003
Rookie Year: 2002
Location: Hauppauge, Long Island, NY
Posts: 8,906
Mark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond repute
Re: printf bug - beware!

Quote:
Originally Posted by deltacoder1020
tsk tsk IFI... this is something you think they would put into the docs... (and if I missed it, oh well, i was speedreading :S)
It all depended on where you looked.

printf_lib.c header:
Code:
* 3. The %s directive only supports a string of 40 bytes and the format string
*	 may not exceed 80 bytes.
The first time we tried to printf a "long" convinced us to toss printf_lib and write our own. Ours took up 40 bytes and worked better, printf_lib took up 4000 bytes. I know many other teams did the same, or rewrote the original printf_lib.
__________________
"Rationality is our distinguishing characteristic - it's what sets us apart from the beasts." - Aristotle

Last edited by Mark McLeod : 08-03-2004 at 14:57.
  #4   Spotlight this post!  
Unread 09-03-2004, 02:00
10intheCrunch's Avatar
10intheCrunch 10intheCrunch is offline
Who's John V-Neun?
AKA: Alex Baxter
None #0254 (Cheesy Poofs)
Team Role: College Student
 
Join Date: Feb 2004
Rookie Year: 2004
Location: San Jose, CA
Posts: 129
10intheCrunch is a jewel in the rough10intheCrunch is a jewel in the rough10intheCrunch is a jewel in the rough10intheCrunch is a jewel in the rough
Send a message via AIM to 10intheCrunch
Re: printf bug - beware!

This isn't a bug either, but causes us and team 60 some amount of frustration over the weekend...

If any printf's are called before the Initialize_Serial_Coms() function is called in User Initialization, the RC will flip into a bizarre state: the disable light goes on and the program light flashes red and green. We could find no documentation on what the signal meant, and spent about 3 hours rebuilding our code before Jay noticed the errant printf at the top of User Initialization.

The problem seems pretty obvious in retrospect, but because of the obscurity of the RC feedback we were getting, I'd just like to warn other teams...don't make the same mistake =).
__________________
~Alex Baxter
Programming, Arms operation, Team 254
  #5   Spotlight this post!  
Unread 09-03-2004, 14:03
Astronouth7303's Avatar
Astronouth7303 Astronouth7303 is offline
Why did I come back?
AKA: Jamie Bliss
FRC #4967 (That ONE Team)
Team Role: Mentor
 
Join Date: Jan 2004
Rookie Year: 2004
Location: Grand Rapids, MI
Posts: 2,071
Astronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud ofAstronouth7303 has much to be proud of
Re: printf bug - beware!

Thank you, not even IFI could tell us what alternating lights ment.
  #6   Spotlight this post!  
Unread 09-03-2004, 17:44
Greg Ross's Avatar
Greg Ross Greg Ross is offline
Grammar Curmudgeon
AKA: gwross
FRC #0330 (Beach 'Bots)
Team Role: Mentor
 
Join Date: Jun 2001
Rookie Year: 1998
Location: Hermosa Beach, CA
Posts: 2,245
Greg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond reputeGreg Ross has a reputation beyond repute
Send a message via AIM to Greg Ross Send a message via Yahoo to Greg Ross
Re: printf bug - beware!

Bill and Mark, if you could share your code, it would be appreciated.
__________________
Greg Ross (The Grammar Curmudgeon formerly known as gwross)
S/W Engineer, Team 330, the Beach 'Bots
<--The Grammar Curmudgeon loves this cartoon.
“Life should not be a journey to the grave with the intention of arriving safely in a pretty and well preserved body, but rather to skid in broadside in a cloud of smoke, thoroughly used up, totally worn out, and loudly proclaiming "Wow! What a Ride!" Hunter S. Thompson
"Playing a practical joke means doing something mean and calling it funny." Me
  #7   Spotlight this post!  
Unread 10-03-2004, 01:50
WizardOfAz's Avatar
WizardOfAz WizardOfAz is offline
Lead Mentor
AKA: Bill Bennett
FRC #1011 (CRUSH)
Team Role: Engineer
 
Join Date: Mar 2003
Rookie Year: 2002
Location: Tucson, AZ
Posts: 101
WizardOfAz will become famous soon enough
Send a message via AIM to WizardOfAz
Re: printf bug - beware!

Quote:
Originally Posted by gwross
Bill and Mark, if you could share your code, it would be appreciated.
OK, happy to do so.

The changes are in three files.

In ifi_utilities.h I added
#define printfBufrLength 80

In ifi_utilities.c I changed the line
char ifi_printfBufr[80];
to
char ifi_printfBufr[printfBufrLength];

Neither of these were really necessary, I could have just coded "80" instead of "printBufrLength" in the changes in printf_lib.c.

This is what is added to printf_lib.c:
the function strlenrom(const rom char * string) counts the length of the string.
strlenrom() is called in several places to compare the length of the source and target.

Some will suggest that I could have used strncpypgm2ram which would limit the amount of the copy with less work. I did it as I did so as to get a clear error on the screen when the function is used improperly, rather than just omit data from the print. I suppose either way is arguable.

Here's the new printf_lib.c code:
Code:
/*******************************************************************************
* FILE NAME: printf_lib.c
* 
* 2004/03/06 (bill bennett) added length check on copy to ifi_printfBufr and to tmpBufr
*
* DESCRIPTION:
*  This file contains generic routines that work like the standard C I/O library.
*  These unsupported routines have been modified to work with the Microchip 
*  C compiler.  The printf routine is limited as follows:
*
*  1.  Only the %s, %lx, %d, %x, %u, %X directives are parsed.  An additional %b 
*      directive is parsed for those who like to view data in binary (base 2).
*
*      Examples:   
*
*            rom const char *StrPtr = "Hello world!";
*            int x = 15;
*            int y = 0x50;
*            long z = 0xdeadface;
*
*            printf("%s\n",StrPtr);                         will display 'Hello world!'
*                                  
*            printf("X = %d, Y = %x, Z = %lx\n",x,y,z);     will display 'X = 15, Y = 0x50, Z = deadface'
*                                                      
*            printf("X = %b (base 2)\n",x);                 will display 'X = 1111 (base 2)'
*                   
*            printf("X = %16b\n",x);                        will display 'X = 0000000000001111'
*                           
*            printf("X = %04x\n",x);                        will display 'X = 000f'
*                           
*  2.  All bytes (8 bits) or bit references (rc_dig_in01) must be type-cast to a  
*      16 bit word.  The %c directive is unsupported.
*
*      Examples:   
*
*            unsigned char byte1 = 0xfa;
*            char byte2 = 25;
*
*            printf("1st byte = %x, 2nd byte = %d\n",(int)byte1,(int)byte2);
*                                                           will display '1st byte = fa, 2nd byte = 25
*            if (rc_dig_in01)
*              printf("On %d\n",(int)rc_dig_in01);          will display 'On 1'
*            else
*              printf("Off %d\n",(int)rc_dig_in01);         will display 'Off 0'
*
*  3.  The %s directive only supports a string of 40 bytes and the format string
*      may not exceed 80 bytes.  The sprintf routine is not included.  Keep in 
*      mind that the stack size has a 256 byte limit.
*
* USAGE:
*  These library routines  may be modified to suit the needs of the user.
*******************************************************************************/

#include "ifi_default.h"
#include "printf_lib.h"
#include "ifi_utilities.h"
#include "string.h"

/* the following should be enough for a 32 bit word */
#define PRINT_BUF_LEN 20

extern char ifi_printfBufr[]; /* declared in ifi_utilities.c */
rom char *nullStr = "(null)";
static const rom char * bufrTooLongMsg = "**** printf format string too long, max is 80 ****\n";
static const rom char * s_formatTooLongMsg = "**** %s arg too long, limit 40 ****"; // make sure this error msg len < 40!

static char print_buf[PRINT_BUF_LEN];
static char scr[2];
static int  k;

unsigned short strlenrom(const rom char * string) 
{
	unsigned short len;
	for(len=0; string[len]!=0; len++) ;
	return len;
}

/*******************************************************************************
* SUBROUTINE NAME: Write_Byte_To_Uart
* PURPOSE:       Writes a byte to the UART.
*     Argument       Type           IO   Description
*     --------       -----------    --   -----------
*         data       int            I    data to transmit to the UART
* RETURNS:       void
*******************************************************************************/

static void Write_Byte_To_Uart(int data)
{
  TXREG = data;  /* a carriage return */
  Wait4TXEmpty();
}

#define PAD_RIGHT 1
#define PAD_ZERO  2

/*******************************************************************************
* FUNCTION NAME: prints
* PURPOSE:       Pads the output stream.
* RETURNS:       void
*******************************************************************************/

static int prints(char *string, int width, int pad)
{
	register int pc = 0, padchar = ' ';

	if (width > 0) {
		register int len = 0;
		register char *ptr;
		for (ptr = string; *ptr; ++ptr) ++len;
		if (len >= width) width = 0;
		else width -= len;
		if (pad & PAD_ZERO) padchar = '0';
	}
	if (!(pad & PAD_RIGHT)) {
		for ( ; width > 0; --width) {
			Write_Byte_To_Uart (padchar);
			++pc;
		}
	}
	for ( ; *string ; ++string) {
		Write_Byte_To_Uart (*string);
		++pc;
	}
	for ( ; width > 0; --width) {
		Write_Byte_To_Uart (padchar);
		++pc;
	}

	return pc;
}

/*******************************************************************************
* FUNCTION NAME: printi
* PURPOSE:       Converts the output stream to the proper width and base.
* RETURNS:       void
*******************************************************************************/

static int printi(int i, int b, int sg, int width, int pad, int letbase)
{
	register char *s;
	register int t, neg = 0, pc = 0;
	register unsigned int u = i;

   print_buf[0] = 0xBE;
   print_buf[PRINT_BUF_LEN] = 0xEF;
	if (i == 0) {
		print_buf[0] = '0';
		print_buf[1] = '\0';
		return prints (print_buf, width, pad);
	}

	if (sg && b == 10 && i < 0) {
		neg = 1;
		u = -i;
	}

	s = print_buf + PRINT_BUF_LEN-1;
	*s = '\0';

	while (u) {
		t = u % b;
		if( t >= 10 )
			t += letbase - '0' - 10;
		*--s = t + '0';
		u /= b;
	}

	if (neg) {
		if( width && (pad & PAD_ZERO) ) {
			Write_Byte_To_Uart ('-');
			++pc;
			--width;
		}
		else {
			*--s = '-';
		}
	}

	return pc + prints (s, width, pad);
}

/*******************************************************************************
* FUNCTION NAME: print
* PURPOSE:       Parses the output stream.
* RETURNS:       void
*******************************************************************************/

static int print(char *format, int *varg)
{
  char tmpBufr[40];
	register int width, pad;
	register int pc = 0;

	for (; *format != 0; ++format) {
		if (*format == '%') {
			++format;
			width = pad = 0;
			if (*format == '\0') break;
			if (*format == '%') goto out;
			if (*format == '-') {
				++format;
				pad = PAD_RIGHT;
			}
			while (*format == '0') {
				++format;
				pad |= PAD_ZERO;
			}
			for ( ; *format >= '0' && *format <= '9'; ++format) {
				width *= 10;
				width += *format - '0';
			}
			if( *format == 's' ) 
      {
		if(strlenrom((rom char *) *varg)<40)
	        strcpypgm2ram(tmpBufr,(rom char *) *varg);
		else
			strcpypgm2ram(tmpBufr, (rom char *) s_formatTooLongMsg);
		varg--;
        if (tmpBufr[0] == 0)
          strcpypgm2ram(tmpBufr,nullStr);
				pc += prints (tmpBufr, width, pad);
				continue;
			}
			if( *format == 'd' ) {
				pc += printi (*varg--, 10, 1, width, pad, 'a');
				continue;
			}
			if( *format == 'x' ) {
				pc += printi (*varg--, 16, 0, width, pad, 'a');
				continue;
			}
			if( *format == 'X' ) {
				pc += printi (*varg--, 16, 0, width, pad, 'A');
				continue;
			}
			if( *format == 'u' ) {
				pc += printi (*varg--, 10, 0, width, pad, 'a');
				continue;
			}
			if( *format == 'l' ) {    /* assumes lx */
				pc += printi (*varg--, 16, 0, width, pad, 'a');
				pc += printi (*varg--, 16, 0, width, pad, 'a');
				format++;               /* skip over x*/
				continue;
			}
			if( *format == 'b' ) {
				pc += printi (*varg--, 2, 0, width, 2, 'a');
				continue;
			}
		}
		else {
		out:
      if (*format == '\n') *format = '\r';  /* replace line feed with cr */
			Write_Byte_To_Uart (*format);
			++pc;
		}
	}
	return pc;
}

/*******************************************************************************
* FUNCTION NAME: printf
* PURPOSE:       Formats an output stream.
* RETURNS:       void
*******************************************************************************/
int printf(rom const char *format, ...)
{
    register int *varg = (int *)(&format);
    /* 
       Since constant strings are kept in program (flash) memory, the strcpypgm2ram
       routine copies the string from flash to ram. 
    */
	if(strlenrom(format)<printfBufrLength) 
	    strcpypgm2ram(ifi_printfBufr,(rom char *) format);
	else 	
		// format string too long, print error message instead
		strcpypgm2ram(ifi_printfBufr, (rom char *)bufrTooLongMsg);

    varg--; /* adjust stack for Microchip C Compiler */
    return print(ifi_printfBufr, varg);
}

/******************************************************************************
* 
* The following routines are examples how to use the printf library routines
* to create your own output modules:
*  
*******************************************************************************/

/*******************************************************************************
* FUNCTION NAME: printid
* PURPOSE:       Prints a 16bit word (base 10) w/o a carriage return.
* RETURNS:       void
*******************************************************************************/

void printid(int data,int crtOn)
{
  printi (data, 10, 1, 4, 2, 'a');
  if (crtOn)
    Write_Byte_To_Uart('\r');
}

/*******************************************************************************
* FUNCTION NAME: printd
* PURPOSE:       Prints an 8bit word (base 10) w/o a carriage return.
* RETURNS:       void
*******************************************************************************/

void printd(unsigned char data,int crtOn)
{
  printi ((int) data, 10, 1, 3, 2, 'a');
  if (crtOn)
    Write_Byte_To_Uart('\r');
}

/*******************************************************************************
* FUNCTION NAME: printib
* PURPOSE:       Prints a 16bit binary word (base 2) w/o a carriage return.
* RETURNS:       void
*******************************************************************************/

void printib(unsigned int data,int crtOn)
{
  printi (data, 2, 0, 16, 2, 'a');
  if (crtOn)
    Write_Byte_To_Uart('\r');
}

/*******************************************************************************
* FUNCTION NAME: printb
* PURPOSE:       Prints an 8bit binary word (base 2) w/o a carriage return.
* RETURNS:       void
*******************************************************************************/

void printb(unsigned char data,int crtOn)
{
  printi ((int) data, 2, 0, 8, 2, 'a');
  if (crtOn)
    Write_Byte_To_Uart('\r');
}

/*******************************************************************************
* FUNCTION NAME: printix
* PURPOSE:       Prints a 16bit hex word (base 16) w/o a carriage return.
* RETURNS:       void
*******************************************************************************/

void printix(int data,int crtOn)
{
  printi (data, 16, 0, 2, 0, 'a');
  if (crtOn)
    Write_Byte_To_Uart('\r');
}

/*******************************************************************************
* FUNCTION NAME: printx
* PURPOSE:       Prints an 8bit hex word (base 16) w/o a carriage return.
* RETURNS:       void
*******************************************************************************/

void printx(unsigned char data,int crtOn)
{
  printi ((int) data, 16, 0, 2, 2, 'a');
  if (crtOn)
    Write_Byte_To_Uart('\r');
}

/*******************************************************************************
* FUNCTION NAME: debug_print
* PURPOSE:       Prints a header and a 16bit hex word (base 16) with a carriage 
*                return.
* RETURNS:       void
*******************************************************************************/

void debug_print(char *bufr,int data)
{
  if(strlenrom(bufr)<printfBufrLength) 
     strcpypgm2ram(ifi_printfBufr,(rom char *) bufr);
  else 	
    // format string too long, print error message instead
	strcpypgm2ram(ifi_printfBufr, (rom char *)bufrTooLongMsg);
  for (k=0;k<strlen(ifi_printfBufr);k++)
    Write_Byte_To_Uart(ifi_printfBufr[k]);
  printix(data,1);
}

/*******************************************************************************
* FUNCTION NAME: debug_printb
* PURPOSE:       Prints a header and an 8bit binary word (base 2) with a carriage 
*                return.
* RETURNS:       void
*******************************************************************************/

void debug_printb(char *bufr,unsigned int data)
{
  if(strlenrom(bufr)<printfBufrLength) 
     strcpypgm2ram(ifi_printfBufr,(rom char *) bufr);
  else 	
    // format string too long, print error message instead
	strcpypgm2ram(ifi_printfBufr, (rom char *)bufrTooLongMsg);
  for (k=0;k<strlen(ifi_printfBufr);k++)
    Write_Byte_To_Uart(ifi_printfBufr[k]);
  printib(data,1);
}

/*******************************************************************************
* FUNCTION NAME: debug_println
* PURPOSE:       Prints a header (assumming a carriage return is supplied).
* RETURNS:       void
*******************************************************************************/

void debug_println(char *bufr)
{
  if(strlenrom(bufr)<printfBufrLength) 
     strcpypgm2ram(ifi_printfBufr,(rom char *) bufr);
  else 	
    // format string too long, print error message instead
	strcpypgm2ram(ifi_printfBufr, (rom char *)bufrTooLongMsg);
  for (k=0;k<strlen(ifi_printfBufr);k++)
    Write_Byte_To_Uart(ifi_printfBufr[k]);
}


/******************************************************************************/
/******************************************************************************/
/******************************************************************************/
  #8   Spotlight this post!  
Unread 10-03-2004, 09:50
Mark McLeod's Avatar
Mark McLeod Mark McLeod is offline
Just Itinerant
AKA: Hey dad...Father...MARK
FRC #0358 (Robotic Eagles)
Team Role: Engineer
 
Join Date: Mar 2003
Rookie Year: 2002
Location: Hauppauge, Long Island, NY
Posts: 8,906
Mark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond reputeMark McLeod has a reputation beyond repute
Re: printf bug - beware!

Quote:
Originally Posted by gwross
Bill and Mark, if you could share your code, it would be appreciated.
Here's mine.

I didn't include some related minor changes made to the ifi_utilities functions.
We added "0x" in front of the hex prints and added a compile flag "_NO_PRINTS", so we could easily turn on/off all prints.

We kept the 80 byte limitation, but truncate strings at 80 bytes. I suppose I could have added "..." or something at the end of a truncated string to indicate when a text string is too long, but this code doesn't create one huge string.
It also doesn't care about the size of the variable being printed. There are separate calls for printing signed vs unsigned integers, and I never paid any attention to floats.
It's inefficient in that it outputs in discrete chunks.

I made it as simple as I could, so the newbie programmers could more easily follow the logic.
Send me any improvements you think of.
Attached Files
File Type: zip prints.zip (1.7 KB, 39 views)
__________________
"Rationality is our distinguishing characteristic - it's what sets us apart from the beasts." - Aristotle

Last edited by Mark McLeod : 10-03-2004 at 17:12.
Closed Thread


Thread Tools
Display Modes Rate This Thread
Rate This Thread:

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
printf and newlines Jeff McCune Programming 5 21-02-2004 21:09
Printf warning during compile? actorindp Programming 4 20-02-2004 17:02
printf isn't printf-ing. Help! Meandmyself Programming 14 15-02-2004 16:27
clear printf box? telefragger2000 Programming 6 05-02-2004 17:40
Last Call for bug reports rbayer Programming 0 08-01-2003 02:48


All times are GMT -5. The time now is 03:07.

The Chief Delphi Forums are sponsored by Innovation First International, Inc.


Powered by vBulletin® Version 3.6.4
Copyright ©2000 - 2017, Jelsoft Enterprises Ltd.
Copyright © Chief Delphi