Thread: What's wrong with my simple C code?

  1. #1
    Registered User
    Join Date
    Jan 2011
    Posts
    6

    What's wrong with my simple C code?

    Hey,

    I have two methods - one for getting input for a string array and the other for printing the array. It's for an assignment for school so I can't change the methods signiture.

    The code:
    Code:
    #include <stdio.h>
    #include <string.h>
    
    #define TOTAL 5
    
    int  Get(char** StrArr,int total);
    void  Display(char** StrArr,int size);
    
    int   main(void)
    {
    	int i;
    
    	char* Strings[TOTAL+1]={NULL};
    	//Get TOTAL dynamic strings from user
    	if (!Get(Strings,TOTAL)) exit(0);
    	//display the array of strings
    	printf("\n\nAfter input, ");
    	Display(Strings,TOTAL);
    }
    
    int Get(char** StrArr,int total)
    {
    	int i, result = 0;;
    	for(i=0;i<total;i++)
    	{
    		printf("string %i:\t", i+1);
    		gets(StrArr+i);
    		if(i==4)
    		{
    			result = 1;
    		}	
    	}
    	return result;
    }
    
    void  Display(char** StrArr,int size)
    {
    	int i;
    	printf("The strings are:\n");
    	for(i=0;i<size;i++)
    	{
    		printf("StrArr is: %p", StrArr+i);
    		printf("%s\n", StrArr+i);
    	}
    }
    When the input is something like:
    aa
    hh
    cc
    ee
    gg
    everything works as expected. When its something more complicated like:
    aaaa
    bb
    ccccc
    hh
    f
    The array doesn't print like it should.
    It shows the first string as "aaaabb" and the third as "ccccf".

    What's wrong with my code

    Thanks!

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Since TOTAL is 5, that means your strings are of length 4 (four characters + '\0' end-of-string character = 5). Any attempt at a string longer than that is doomed to failure.

    This is why you should never use gets -- you need to use an input mechanism that takes into account how much room you actually have to store things (such as fgets).

  3. #3
    Registered User
    Join Date
    Jan 2011
    Posts
    6
    I was sure that the TOTAL indicates the number of strings.
    Not their lenght.

    scanf("%s",StrArr+i) doesn't work either.
    I don't know what will be the size of the input.

    One of the requirements in this exercise is that each string will take only the amount of room it actually needs - not more.

    How can I solve this?

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Oh yes, you're right. I was reading as char[], not char*[].

    In that case, you have room for exactly 0 letters in each string, which is even worse. You need to malloc some memory for each of those pointers to point to. If you are required to not go over, then you are in the unenviable position of having to malloc a single byte, and then keep realloc'ing one byte more every time you read a character.

  5. #5
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    1. Never use gets. Use fgets instead.
    2. You aren't currently allocating any memory to hold the strings.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  6. #6
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by godmoney View Post
    I was sure that the TOTAL indicates the number of strings.
    Not their lenght.

    scanf("%s",StrArr+i) doesn't work either.
    I don't know what will be the size of the input.

    One of the requirements in this exercise is that each string will take only the amount of room it actually needs - not more.

    How can I solve this?
    Ok then you need to use malloc to create an array of pointers to strings then allocate a buffer of some ungodly size that's guaranteed to be bigger than any string your user will input... Let the user feed the buffer then use malloc again to allocate space for each string and add it to the array.... Then once the array is completed you can do what you need to it...

    As it is now you have allocated only 5 bytes for string storage.

  7. #7
    Registered User
    Join Date
    Jan 2011
    Posts
    6
    I don't have much experience coding in C... can you give me a code example or something so I'll understand what you are talking about ?

  8. #8
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Then it looks like you're going to get some experience with malloc.

  9. #9
    Registered User
    Join Date
    Jan 2011
    Posts
    6
    Cool - I'm all up for learning.
    I just need to understand the what and how.

  10. #10

  11. #11
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by godmoney View Post
    Cool - I'm all up for learning.
    I just need to understand the what and how.
    Well, I'd suggest you try to sort it out on your own... when you run into troubles, post your code and lets see what we can do...

    You already have the basics...

    1) Create a text buffer for raw user input
    2) Create an array of pointers to strings of suitable size
    3) clear the text buffer
    4) get the user's input
    5) calculate the length of the user input
    6) allocate memory in the array from #2 based on #5
    7) copy the string from the buffer to the array
    8) loop to step 3 until done.

  12. #12
    Registered User
    Join Date
    Jan 2011
    Posts
    6
    o.k. - let's see if I got this right:

    1) Create a text buffer for raw user input - not sure what you mean by that
    2) Create an array of pointers to strings of suitable size - I guess that
    Code:
    char* Strings[TOTAL+1]={NULL};
    covers it.
    3) clear the text buffer - not sure how to do that. I've read in some website about the flushall() function but my compiler doesn't recognize it.
    4) get the user's input - simple
    Code:
    scanf("%s",StrArr+i)
    would do?
    5) calculate the length of the user input - how? didn't I already store the information from the user?
    6) allocate memory in the array from #2 based on #5 - I know I'm supposed to use mallock but not sure how in this case
    7) copy the string from the buffer to the array - I guess it's a simple step once all is done
    8) loop to step 3 until done. - looks pretty easy

  13. #13
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by godmoney View Post
    o.k. - let's see if I got this right:

    1) Create a text buffer for raw user input - not sure what you mean by that
    2) Create an array of pointers to strings of suitable size - I guess that
    Code:
    char* Strings[TOTAL+1]={NULL};
    covers it.
    3) clear the text buffer - not sure how to do that. I've read in some website about the flushall() function but my compiler doesn't recognize it.
    4) get the user's input - simple
    Code:
    scanf("%s",StrArr+i)
    would do?
    5) calculate the length of the user input - how? didn't I already store the information from the user?
    6) allocate memory in the array from #2 based on #5 - I know I'm supposed to use mallock but not sure how in this case
    7) copy the string from the buffer to the array - I guess it's a simple step once all is done
    8) loop to step 3 until done. - looks pretty easy
    Ok... in C there really is no such thing as a string. It's really just an array of characters with a 0 at the end. These arrays are not magic, you have to create and manipulate them on your own.
    For example:
    Code:
    // static array
    char Name[48];
    
    puts ("Enter your name please");
    scanf("%s",Name);
    
    // dynamic array
    char *Name;
    
    Name = malloc(48 * sizeof(char));
    
    puts ("Enter your name please");
    scanf("%s",Name);
    Either of these methods allow up to 47 characters to be entered as the user's name. The extra character is to hold the trailing null.

    What you're not doing in your program is making space for strings...

  14. #14
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Quote Originally Posted by CommonTater View Post
    Either of these methods allow up to 47 characters to be entered as the user's name. The extra character is to hold the trailing null.
    Technically, either of those methods allow you to enter as many as you try, so long as you aren't including white space. There is nothing actually limiting the number of characters you can type. Since we're all nit picking here, I figured I might as well also.


    Quzah.
    Hope is the first step on the road to disappointment.

  15. #15
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by quzah View Post
    Technically, either of those methods allow you to enter as many as you try, so long as you aren't including white space. There is nothing actually limiting the number of characters you can type. Since we're all nit picking here, I figured I might as well also.


    Quzah.
    LOL... are we happy now?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. whats wrong with this very simple code?
    By sway1 in forum C++ Programming
    Replies: 2
    Last Post: 07-06-2008, 12:18 PM
  2. What's wrong with this code?
    By Luciferek in forum C++ Programming
    Replies: 4
    Last Post: 06-21-2008, 12:02 PM
  3. Whats wrong with this simple code?
    By o14v in forum C++ Programming
    Replies: 4
    Last Post: 03-19-2008, 01:46 PM
  4. What is wrong with my code? My first program......
    By coreyt1111 in forum C++ Programming
    Replies: 11
    Last Post: 11-14-2006, 02:03 PM