1

I make a test case about putting input to a char pointer. When I try to run this program, the output isn't right(it becomes a series of random characters like : _ @$).I intend to print each element in that char pointer. I make some changes on my code, but it's still wrong(same problem as before). Could someone help me to figure out what's going wrong and the way to fix it?

int chara;
int counts =0;

main(){

    char *buffer=(char *)malloc(sizeof(char)*25);
    while((chara=getchar())!= EOF&& counts<25){
        *buffer++ = chara;
        printf("%c\n",*buffer);
        counts++;

    }
    *buffer = '\0';
     printf("%s\n",buffer);
     free(buffer);
}
Arabeka
  • 191
  • 1
  • 1
  • 6
  • `sizeof(20) == 4`. That's not what you want. – SLaks Oct 08 '13 at 16:38
  • `sizeof(20)` doesn't do what you eant. You want `malloc(20)` if you want to allocate 20 bytes. Also, please be specific. Saying "the output isn't right" makes it difficult to help. – lurker Oct 08 '13 at 16:38
  • You should add something to your loop to stop it when you get all but one byte of the buffer filled. The one you have will run forever, overwriting whatever is after the buffer (most likely). – Lee Meador Oct 08 '13 at 16:39
  • `getchar` returns `EOF` when it gets to the end of input from the user (with `^D`). So your loop condition is wrong. – lurker Oct 08 '13 at 16:40
  • Also, this one is so obvious a problem that we let you get by with a badly worded question. You will need, in the future, to show the input to the program and the output. Then explain what is wrong with the output or what you expected it to be. Then make sure you ask a question about the problem. "Could someone help ...?" isn't a question about the problem. It's a question about your situation and about Stack Overflow. – Lee Meador Oct 08 '13 at 16:41
  • [Don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). – unwind Oct 08 '13 at 16:58
  • 1
    The code currently in the question writes one byte beyond the end of the allocated buffer if the input is longer than 25 characters. Your in-loop printing code prints the character in the buffer that you've not yet assigned to, which is why you get random-ish characters out. – Jonathan Leffler Oct 08 '13 at 17:11
  • Here I add some conditions to avoid stack overflow, but this program doesn't print the values I put in the buffer as expected. – Arabeka Oct 08 '13 at 17:11
  • You are printing the non-character after the one you just put in the buffer because you `++` after storing the char and before printing it. – Lee Meador Oct 08 '13 at 19:07

3 Answers3

2

Modify your code.

    int chara; //getchar() returns int
    int i=0;
    char *buffer=malloc(20); //allocate correctly
    while(  ( (chara=getchar())!= EOF) && ( i!=19 ) ) { //check against EOF and check counter value to avoid input with legth greater than allocated size.
        buffer[i]= chara;   
        //use indexing with counter variable to avoid errors with free()
        // if change pointer you can't free() memory  
         printf("%c\n",buffer[i++]);
    }

    buffer[i] = '\0';
     printf("%s\n",buffer);
     free(buffer);
Gangadhar
  • 10,248
  • 3
  • 31
  • 50
  • 1
    Still need a counter to make sure the buffer isn't overflowed if the input is longer than 19 chars. – Lee Meador Oct 08 '13 at 16:42
  • Indeed.added counter checking. – Gangadhar Oct 08 '13 at 17:06
  • well, even if added counter, it still doesn't print values in char pointer as what it expected to do. The output is just random characters – Arabeka Oct 08 '13 at 17:18
  • 1
    gangadhar: Oh, yeah, use a counter to increment char pointer instead of doing pointer++,thank you – Arabeka Oct 08 '13 at 18:08
  • I would use `i < 19` which would be unnecessary in this case but a better habit to get into. I'd also put the `i++` on its own line because people comment out debugging type print lines and that would break it. But it does solve the issue. – Lee Meador Oct 08 '13 at 19:06
2

Since you increase the value of buffer in the loop, the code after the loop is wrong. You can't free() the incremented buffer, that will point at an address which hasn't been returned by malloc().

Basically you're doing:

char *buffer = malloc(25);
...
free(buffer + length of string the user entered);

Which means that the address you pass to free() is no longer the same that was returned by malloc(), which is an error.

unwind
  • 391,730
  • 64
  • 469
  • 606
  • I don't get it, I think free() just free the memory I allocate for the buffer. Why does it point to the address which is not returned by malloc()? – Arabeka Oct 08 '13 at 17:09
  • Because you keep incrementing `buffer` so it is not pointing at the start of the memory -- and what you free must be the address returned by malloc. – Jonathan Leffler Oct 08 '13 at 17:17
0

Your other problem is that this:

    *buffer++ = chara;
    printf("%c\n",*buffer);

prints out garbage -- you assign chara to where buffer is pointing, increment buffer, and then print out the (uninitialized) location that buffer now points to.

Just print chara instead.

David Gelhar
  • 27,873
  • 3
  • 67
  • 84