Incorrect format using printf()

This is a discussion on Incorrect format using printf() within the C Programming forums, part of the General Programming Boards category; For the past several hours, I have been trying to solve a formatting bug in my code. I have commented ...

  1. #1
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198

    Unhappy Incorrect format using printf()

    For the past several hours, I have been trying to solve a formatting bug in my code. I have commented out a lot of code to trace the root cause.

    I'm using printf("%s %s") to print out the first and last name. With this construction
    if I input "Tom Hanks" I expected that to be printed out as so.

    What I'm getting is:

    Tom
    _Hanks

    I suspected a newline and a space being stuck in the first position of workers.lname. I used the strlen() function to count the number of chars after both fname and lname, and counting the '\0' I get the exact number as expected.

    I've also spent a lot of time in Prelude's corner. Everything there makes sense.

    Cprogramming.com FAQ > Why gets() is bad / Buffer Overflows

    here's my code:

    Code:
    //		9/14/10
    //
    // practice using bit fields inside structures
    //
    
    #include <stdio.h>
    #include <string.h>
    
    #define YES 1;
    #define NO 0;
    
    struct emp_data
    {
    	unsigned char	
    		dental	: 1,
    		college	: 1,
    		health	: 2;
    	char fname[20];
    	char lname[20];
    	char ssnumber[12];
    };
    
    struct emp_data workers[100];
    
    
    int main( void )
    {
        
    
    //	int count;
    //	size_t length;
    
    //	for( count=0; count<1; count++ )
    //	{
    		printf("Enter first name: ");
    		fgets(workers[0].fname, sizeof(workers[0].fname), stdin);
    
    		printf("Enter last name: ");
    		fgets(workers[0].lname, sizeof(workers[0].lname), stdin);
    
    
    		printf("\nDental?");
    		workers[0].dental = 1;
    
    //		scanf("%u", workers[count].dental);		I plan to replace scanf() with another function.
    	
    		printf("\nCollege?");
    		workers[0].college = 1;
    
    //		scanf("%u", workers[count].college);
    	
    			printf("\nHealth?");
    			workers[0].health = 3;
    
    //			scanf("%u", workers[count].health);
    
    			
    		printf("\nEnter SS Number:");
    		fgets(workers[0].ssnumber, sizeof(workers[0].ssnumber), stdin);
    
    		printf("\n\n%s %s", workers[0].fname, workers[0].lname);
    
    
    		printf("dental= %d\tCollege= %d\tHealth= %d\tSS Number= %s\n\n", 
    				workers[0].dental, workers[0].college, workers[0].health, workers[0].ssnumber);
    
    //				while (getchar() != '\n');	// code Hammer's code from Prelude's corner
    					
    //	}
    
         return 0;
    }
    Any suggestions always appreciated.

  2. #2
    Just a pushpin. bernt's Avatar
    Join Date
    May 2009
    Posts
    426
    I suspected a newline and a space being stuck in the first position of workers.lname. I used the strlen() function to count the number of chars after both fname and lname, and counting the '\0' I get the exact number as expected.
    fgets does include a newline at the end of the string. And the format of your printf: "\n\n%s %s" (I bolded the space but you can't see that ) accounts for the leading space in the second line.

    EDIT: But don't get rid of the space; I was just mentioning that part.
    And strlen doesn't count the \0.
    Consider this post signed

  3. #3
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,420
    And all
    scanf("%u", workers[count].health);
    should be
    scanf("%u", &workers[count].health);
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  4. #4
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198

    Unhappy Passing Pointer to Struct Problem

    My program after modifications ran perfectly until I decided to upgrade it with the use
    of a print_records function. If I'm able to fix that, I'd like to pull the code out of main that
    creates the records into another function.

    When I create the pointer to a type struct, I'm using a very similar example from
    "Teach yourself C in 21 Days". 6th ed. 2004.

    Has that much changed in the C world since 2004? Also, this stupid book near the last
    chapter gave a 1 page example of what a bit field looks like inside a structure. I figured out
    by myself how to us it.

    The error that I'm getting is: Error 1 error C2040: 'pemp' : 'int' differs in levels of indirection from 'emp_data *'

    I think I'm really close. I'm finishing this book tonight; I have the latest C Primer Plus next
    to me that I've been drooling over the past 3 weeks. Using the teach yourself C in 21 has
    been a love/hate relationship. More hate than love. I'll probably use it for kindling at tomorrow's barbecue.

    The problem area is highlighted in red. Thanks for help!

    Code:
    //		9/14/10
    //
    // practice using bit fields inside structures
    // while at the same time using Cprogramming.com approved I/O functions.
    //
    
    #include <stdio.h>
    #include <string.h>
    
    #define YES 1;
    #define NO 0;
    #define MAX 10  // set upper limit for # records here.
    
    struct emp_data
    {
    		unsigned char	
    			health	  : 2,	  // width for 4 possible types of health coverage
    			dental	  : 1,
    			college	  : 1,
    			hen_house : 1;	// turns the light on in the chicken coop
    	char fname[20];
    	char lname[20];
    	char ssnumber[12];
    };
    
    struct emp_data workers[MAX];
    
    struct emp_data *pemp;	// create a pointer to type emp_data
    
    pemp = workers;	// Initialize the pointer to workers first struct array[0]
    
    void print_records( struct emp_data *qptr, int );
    
    
    int main( void )
    {
        void *ptr;	// declaring a generic pointer 
    
    	
    
    	unsigned bit;
    	int count;
    
    	for( count=0; count<MAX; count++ )
    	{
    		workers[count].dental = NO;	// initialize all bit fields
    		workers[count].college = NO;
    		workers[count].health = NO;
    
    		printf("\nEnter first name: ");
    
    		ptr = fgets(workers[count].fname, sizeof(workers[count].fname), stdin);
    		if ( ptr == NULL )
    		{
    			printf("Error entering fname\n\n");	// Funtionality tested OK when entered cntrl^z
    			return 1;
    		}
    
    		printf("Enter last name: ");
    
    		ptr = fgets(workers[count].lname, sizeof(workers[count].lname), stdin);
    		if ( ptr == NULL )
    		{
    			printf("Error entering lname\n\n");	// Abort if error
    			return 1;
    		}
    
    		printf("\nDental?");
    
    		scanf("%u", &bit );
    		if( bit == 1 )
    		{
    			workers[count].dental = YES;
    		}
    	
    		printf("\nCollege?");
    
    		scanf("%u", &bit );
    		if( bit == 1 )
    		{
    			workers[count].college = YES;
    		}
    
    			while (getchar() != '\n');	// <--- why did I have to resort to this??
    
    			printf("\nHealth?");
    			scanf("%u", &bit );		// toggle the bit fields
    
    			switch ( bit)
    			{
    			case 0:
    				{
    					workers[count].health |= bit;
    					break;
    				}
    			case 1:
    				{
    					workers[count].health |= bit;
    					break;
    				}
    			case 2:
    				{
    					workers[count].health |= bit;
    					break;
    				}
    			case 3:
    				{
    					workers[count].health |= bit;
    					break;
    				}
    			default:
    				{
    					printf("Invalid entry for health\n");
    					break;
    				}
    			}
    			
    		printf("\nEnter SS Number:\n");
    
    		while (getchar() != '\n');	// clear stdin - Hammer's code from Prelude's corner
    
    		ptr = fgets(workers[count].ssnumber, sizeof(workers[count].ssnumber), stdin);
    		if ( ptr == NULL )
    		{
    			printf("Error entering ssnumber\n\n");	// Abort if error
    			return 1;
    		}
    
    		while (getchar() != '\n');	// clear stdin
    	}
    
    	print_records( pemp, MAX );  // print out all records
         return 0;
    }
    
    void print_records( struct emp_data *sptr, int size )
    {
    	int count;
    
    	for( count=0; count<size; count++ )	// print out the records entered
    		{
    			printf("\n\n%s%s", workers[count].fname, workers[count].lname);	// will use sptr when repairs are made
    
    			printf("dental= %d\tCollege= %d\tHealth= %d\tSS Number= %s\n\n", 
    				workers[count].dental, workers[count].college, workers[count].health, workers[count].ssnumber);
    
    			sptr++;  // point to the next structure
    
    			printf("\n");
    		}
    }
    Last edited by Char*Pntr; 09-14-2010 at 10:53 PM. Reason: Forgot to add MAX

  5. #5
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198
    I forgot to thank Bernt and Elysia for your help. Bernt: The lame book I described above, did NOT mention that a newline was added after fgets() did its thing. I was staring at my printf() code and after your reply, I picked up a book that I just bought by Samuel Harbison III - C A Reference Manual... and inside there... sure enough the last thing fgets() does is add a newline if there still room. Thanks :-)

  6. #6
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,420
    C changed much? You have to be kidding. C moves at a snail's pace. The next standard will literally bring next to nothing new to C after over 10 years of work.
    Anyway, the problem is that you're assigning a pointer outside main. You can't do that. You must initialize it. That means you put "=" after the declaration and a valid value after.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  7. #7
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198

    Smile

    Quote Originally Posted by Elysia View Post
    C changed much? You have to be kidding. C moves at a snail's pace. The next standard will literally bring next to nothing new to C after over 10 years of work.
    Anyway, the problem is that you're assigning a pointer outside main. You can't do that. You must initialize it. That means you put "=" after the declaration and a valid value after.
    Holy Molely! I just compiled it with no erors.

    here a snip:

    Code:
    struct emp_data workers[10];
    
    void print_records( struct emp_data *qptr, int );
    
    
    int main( void )
    {
        void *ptr;	// declaring a generic pointer 
    
    	struct emp_data *pemp = workers;	// create a pointer to type emp_data
    //	pemp = workers;	// Initialize the pointer to workers first struct array[0]
    
    	unsigned bit;
    	int count;
    
    	for( count=0; count<MAX; count++ )
    I'll figure it out later why this had to be inside main() when a similar example was in
    my book that did everything above main()... but why does everything have to be initialized on the same line?

    Please check out what I did... the program seems to be running now, thank you Elysia.

    Oh, and if you think this Teach yourself C in 21 days is a good book, I'll be happy to give it to you!

  8. #8
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,420
    Separate initialization and assignment.

    void* p = NULL; // Initialization

    void* p;
    p = NULL; // Assignment

    Initialization can be done outside main; assignment cannot.

    Quote Originally Posted by Char*Pntr View Post
    Oh, and if you think this Teach yourself C in 21 days is a good book, I'll be happy to give it to you!
    That's a joke, right?
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  9. #9
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    4,165
    That's a joke, right?
    Obviously. No one in their right mind would have that book.

    Soma

  10. #10
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198
    Quote Originally Posted by phantomotap View Post
    Obviously. No one in their right mind would have that book.

    Soma
    Duly noted.


    Quote Originally Posted by Elysia View Post
    Separate initialization and assignment.

    void* p = NULL; // Initialization

    void* p;
    p = NULL; // Assignment

    Initialization can be done outside main; assignment cannot.

    That's a joke, right?
    Yes that was a joke. :-) Well, the important thing that I learned from all of
    this was the initialization and assignment concepts. Thanks for your help!

  11. #11
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198

    Cool fgets() solutions instead of using scanf()

    I originally started out with just a self taught lesson on manipulating bit fields, but
    instead most of my work was getting weaned off of the scanf() when reading strings.

    When I moved the print_records and enter_records to their own function, now there's
    hardly anything left in main()!

    I got an unexpected bonus: I'm going to extract the fgets() reads and create a function
    to read in strings in all of my future programs.

    I've tried to break this code for overflow, etc. Special thanks to Kermit also, I did a
    search on the scanf() problem and he posted a link on another thread. So I used that
    and bookmarked.

    Here's my updated code. Any comments always welcomed.

    Code:
    //		9/15/10
    //
    // practice using bit fields inside structures
    // while at the same time using Cprogramming.com approved I/O functions.
    //
    // As it turned out, 95% of my effort was in dealing with stdin & replacement functions
    // for scanf() and gets().
    // http://faq.cprogramming.com/cgi-bin/smartfaq.cgi?answer=1044652485&id=1043284385
    //
    
    #include <stdio.h>
    #include <string.h>
    
    #define YES 1;
    #define NO 0;
    #define MAX 5  // <--- set upper limit for # records here.
    #define MAX_NAME 20
    #define MAX_SSNMBR 12
    
    struct emp_data
    {
    		unsigned char	
    			health	  : 2,	  // width for 4 possible types of health coverage
    			dental	  : 1,
    			college	  : 1;
    	char fname[MAX_NAME];
    	char lname[MAX_NAME];
    	char ssnumber[MAX_SSNMBR];
    };
    
    struct emp_data workers[MAX];
    
    int enter_records( struct emp_data *, int );	// enters data into employee records
    void print_records( struct emp_data *, int );	// prints out all employee records
    
    
    int main( void )
    {
    	struct emp_data *pemp = workers;	// create a pointer to type emp_data & initialize
    	int return_code;
    
    	return_code = enter_records( pemp, MAX );
    
    	if (return_code != 0)
    		return 1;	// Error msg already printed out
    
    	print_records( pemp, MAX ); 
    
        return 0;
    }
    
    
    int enter_records( struct emp_data *qptr, int max)
    {
    	void *ptr;	// declaring a generic pointer 
    
    	unsigned bit;
    	char *p;
    	int count;
    	char buf[BUFSIZ];
    	size_t name_max = MAX_NAME;
    	size_t ssnumbr_max = MAX_SSNMBR;
    
    	for( count=0; count<max; count++ )
    	{
    		qptr->dental = NO;	// initialize all bit fields
    		qptr->college = NO;
    		qptr->health = NO;
    
    		{	
    			printf("\nEnter first name: ");
    
    			ptr = fgets(buf, BUFSIZ, stdin);	// enters string from stdin
    			if ( ptr == NULL )		// check for error
    			{
    				printf("Error entering fname\n\n");	// Funtionality tested OK when entered cntrl^z
    				return 1;
    			}
    			if ((p = strchr(buf, '\n')) != NULL)  // get rid of that '\n' from the FAQ
    				*p = '\0';	
    			strncpy( qptr->fname, buf, name_max - 1 );	// only copys name_max-1 chars to destination from buf.
    		}
    
    		{
    			printf("Enter last name: ");
    
    			ptr = fgets(buf, BUFSIZ, stdin);
    			if ( ptr == NULL )
    			{
    				printf("Error entering lname\n\n");	// Abort if error
    				return 1;
    			}
    			if ((p = strchr(buf, '\n')) != NULL)
    				*p = '\0';	
    			strncpy( qptr->lname, buf, name_max - 1 );
    		}
    
    		do 
    		{
    			printf("\nEnter Dental 1 or 0\n");	// toggle the 1-bit fields
    			scanf("%u", &bit );
    			if( bit == 1 )
    				qptr->dental = YES;	
    		} while ( bit > 1 );		
    
    		do
    		{
    			printf("\nEnter College 1 or 0\n");
    			scanf("%u", &bit );
    			if( bit == 1 )
    			{
    				qptr->college = YES;
    			} 
    		} while ( bit > 1 );
    
    		do
    		{
    			printf("\nEnter Health 0 - 3\n");	// 2-bit field
    			scanf("%u", &bit );		
    
    			switch ( bit)	// determine which health bits to set
    			{
    			case 0:
    				  {
    					qptr->health |= bit;
    					break;
    				  }
    			case 1:
    				  {
    					qptr->health |= bit;
    					break;
    				  }
    			case 2:
    				  {
    					qptr->health |= bit;
    					break;
    				  }
    			case 3:
    				  {
    					qptr->health |= bit;
    					break;
    				  }
    			default:
    				  {
    					printf("Invalid entry for health\n");
    				  }
    			}
    
    		} while ( bit > 3 );
    			
    			{
    				while (getchar() != '\n');	// clear stdin newline
    
    				printf("\nEnter SS Number:\n");
    
    				ptr = fgets(buf, BUFSIZ, stdin);
    				if ( ptr == NULL )
    				{
    					printf("Error entering ssnumber\n\n");	// Abort if error
    					return 1;
    				}
    				if ((p = strchr(buf, '\n')) != NULL)
    					*p = '\0';	
    				strncpy( qptr->ssnumber, buf, ssnumbr_max - 1 );
    			}
    
    		qptr++; // point to the next empty data record
    	}
    
    	return 0;
    }
    
    
    void print_records( struct emp_data *sptr, int size )
    {
    	int count;
    
    	for( count=0; count<size; count++ )	// print out the records entered
    		{
    			printf("\n\n%s %s\n", sptr->fname, sptr->lname);
    
    			printf("dental= %d\tCollege= %d\tHealth= %d\tSS Number= %s\n", 
    				sptr->dental, sptr->college, sptr->health, sptr->ssnumber);
    
    			sptr++;  // point to the next data record
    		}
    
    	printf("\n");
    }

  12. #12
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,046
    My comments here are mostly matters of style, they won't help you too much. You may want to just read over them, consider them, and possibly implement them next time.

    I'm not too clear on what you're trying to do with that switch statement. I'm pretty sure that whatever it is, there's an easier way. All of the cases seem to be identical, so I'd like to point out in case you were unaware of it that you can also do this:
    Code:
                switch ( bit)    // determine which health bits to set
                {
                case 0:
                case 1:
                case 2:
                case 3:
                      {
                        qptr->health |= bit;
                        break;
                      }
                default:
                      {
                        printf("Invalid entry for health\n");
                      }
                }
    In other words, since the code continues executing until it hits a breakpoint, you can use it to your advantage to combine switch cases together.

    Another comment: you use quite a few anonymous blocks in your code (it's something I do a lot of as well ), which can be good but in your case it sometimes is just a way to separate out some code that should really be put into a function. For example, both of these would make good functions.
    Code:
            do 
            {
                printf("\nEnter Dental 1 or 0\n");    // toggle the 1-bit fields
                scanf("%u", &bit );
                if( bit == 1 )
                    qptr->dental = YES;    
            } while ( bit > 1 );
    Code:
            {    
                printf("\nEnter first name: ");
    
                ptr = fgets(buf, BUFSIZ, stdin);    // enters string from stdin
                if ( ptr == NULL )        // check for error
                {
                    printf("Error entering fname\n\n");    // Funtionality tested OK when entered cntrl^z
                    return 1;
                }
                if ((p = strchr(buf, '\n')) != NULL)  // get rid of that '\n' from the FAQ
                    *p = '\0';    
                strncpy( qptr->fname, buf, name_max - 1 );    // only copys name_max-1 chars to destination from buf.
            }
    Yes there is some code that is different, but you can easily pass in enough information to make a generic solution.

    Lastly, fgets() actually returns a pointer of type char* (see the man page) -- although if you don't care about the type assigning it to a void* pointer is as good as anything. But your use of ptr could easily be combined into one line:
    Code:
                    if ( fgets(buf, BUFSIZ, stdin) == NULL )
    Personally I find that easier to read, but again that's just a matter of style.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  13. #13
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198

    Smile

    Quote Originally Posted by dwks View Post
    My comments here are mostly matters of style, they won't help you too much. You may want to just read over them, consider them, and possibly implement them next time.

    I'm not too clear on what you're trying to do with that switch statement. I'm pretty sure that whatever it is, there's an easier way. All of the cases seem to be identical, so I'd like to point out in case you were unaware of it that you can also do this:
    Code:
                switch ( bit)    // determine which health bits to set
                {
                case 0:
                case 1:
                case 2:
                case 3:
                      {
                        qptr->health |= bit;
                        break;
                      }
                default:
                      {
                        printf("Invalid entry for health\n");
                      }
                }
    In other words, since the code continues executing until it hits a breakpoint, you can use it to your advantage to combine switch cases together.
    Thank you for your suggestions dwks! I had a bad case of programming tunnel vision. I'm dealing with individual bits, and my first reaction was to deal with the different settings for the 2-bit field individually.

    It turns out, I eliminated the switch completely. I just ran this code below with no problems:

    Code:
    do
    		{
    			printf("\nEnter Health 0 - 3\n");	// 2-bit field
    			scanf("%u", &bit );		
    
    			if ( bit < 4 )
    			{
    				qptr->health |= bit;
    				continue;
    			}				
    			printf("Invalid entry for health\n");
    
    		} while ( bit > 3 );
    I'm now looking at your other suggestions. Since my last post, I wrote a generic function
    that I can use for now on to replace scanf() with strings.

    I've already pulled out the fgets() block in my read_record file, and wrote a new function
    prototype: char *read_string( void )

    I've already tested it and it seems to work fine. Here's my new function:

    Code:
    char *read_string()
    {
    	char *q, *str;
    	char buf[BUFSIZ];
    
    	str = fgets(buf, BUFSIZ, stdin);
    			if ( str == NULL )
    			{
    				printf("Error entering lname\n\n");	// Abort if error
    				return NULL;
    			}
    			if ((q = strchr(buf, '\n')) != NULL)  // deletes newline at end of buf
    				*q = '\0';
    	return str;
    }
    Now, I want to include more code using a macro isspace() which will remove all leading
    and trailing white space from the string buffer. This will be a little more tricky, but with pen and paper ready, I'll find a suitable algorithm.

    Before my main got smaller by using functions, now my enter_records is getting smaller as I find more routines that could be written as a function. Thanks again dwks!

  14. #14
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,046
    Be very careful with your read_string() function. There's a bug in it. If the read was successful, you return str, which is really buf since that's what fgets() returns on success. But buf is a local variable allocated on the stack, so when you use the returned value from read_string() you'll be accessing a variable that's out of scope. There's an interesting discussion of out-of-scope variables going on at the moment over in the C++ board: Where do they go?

    The main issue: the data is still there, sitting on the stack, which is why the code seems to work; but because it's gone out of scope, you're officially not supposed to be accessing it anymore, and the operating system could reclaim it at any time. Furthermore, if you call another function it's more than likely to start overwriting some of the data on the stack and messing up your string.

    Basically, you have three options for a read_string() function. You can:
    1. Make buf a static variable. This is the easiest fix but it's a really bad idea, because if you call read_string() multiple times it will start overwriting the same buffer, which can lead to some nasty bugs.
    2. Declare buf[] outside read_string() and instead pass it in as a parameter. This ensures that buf will continue to exist after read_string() returns. I would recommend this solution.
    3. malloc() some memory in read_string(), put fgets()'s data into that buffer, and then return the malloc()'d memory. This is a somewhat more advanced option; it comes with some trickiness, since you have to remember to free() the data eventually (or else you'll have a memory leak). It's probably what I would use but if you're just starting out with functions I'd go with option 2.


    BTW, if you go with the last option and you're feeling adventurous you can write a read_string() function that doesn't have a cap on the maximum length of a line that the user enters. Just combine realloc() with fgets() in a loop . . . .

    Have fun.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  15. #15
    Registered User Char*Pntr's Avatar
    Join Date
    Sep 2007
    Location
    Lathrop, CA
    Posts
    198

    Smile

    Quote Originally Posted by dwks View Post
    Be very careful with your read_string() function. There's a bug in it. If the read was successful, you return str, which is really buf since that's what fgets() returns on success. But buf is a local variable allocated on the stack, so when you use the returned value from read_string() you'll be accessing a variable that's out of scope. There's an interesting discussion of out-of-scope variables going on at the moment over in the C++ board: Where do they go?
    thank you very much dwks, I will read up on this issue.

    [*]Declare buf[] outside read_string() and instead pass it in as a parameter. This ensures that buf will continue to exist after read_string() returns. I would recommend this solution.
    Actually I thought of this, but did not implement it that way, because I wanted to write
    my function with no arguments, that simply returns a pointer. The implementation looked sleek.

    [*]malloc() some memory in read_string(), put fgets()'s data into that buffer, and then return the malloc()'d memory. This is a somewhat more advanced option; it comes with some trickiness, since you have to remember to free() the data eventually (or else you'll have a memory leak). It's probably what I would use but if you're just starting out with functions I'd go with option 2.
    Your 3rd suggestion is the one I'd like to do. Why? because it's more challenging.


    BTW, if you go with the last option and you're feeling adventurous you can write a read_string() function that doesn't have a cap on the maximum length of a line that the user enters. Just combine realloc() with fgets() in a loop . . . .
    I'll keep this in mind! However, I couldn't imagine a situation where the string entered from the keyboard would need to be so large. At this point, FILE *fp operations would probably be more suitable.

    Again, the most important thing is for me to be able to replace scanf() when reading strings, in my code with my own solution. That way, in the future when I ask for help here for some problem, the problem at hand will be easily addressed, and I wont have to waste other peoples time with scanf(), gets() etc. issues.

    Again I sure appreciate all your help. :-)

    Edit:
    This thread is complete. Please feel free to IM me for other comments. :-)
    Last edited by Char*Pntr; 09-17-2010 at 10:00 PM.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Help again!!
    By ewic0190 in forum C Programming
    Replies: 1
    Last Post: 03-19-2010, 06:56 PM
  2. Help with a C Programing Quiz project
    By dilemma in forum C Programming
    Replies: 12
    Last Post: 05-15-2009, 03:35 PM
  3. segmentation fault upon reload
    By yabud in forum C Programming
    Replies: 8
    Last Post: 12-18-2006, 05:54 AM
  4. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 01:27 PM
  5. Azbia - a simple RPG game code
    By Unregistered in forum Game Programming
    Replies: 11
    Last Post: 05-03-2002, 06:59 PM

Tags for this Thread


1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21