0
#include <stdio.h>
#include <string.h>

int * bin(char a);

int main(void){
        char a='a';
        int k=0;
        int *binary;

        binary=bin(a);
        for(k=0; k<8; k++){
                printf("%d", *binary++); 
                printf("\n");      
        }

        return 0;
}
int *bin(char a){

        int i=0;
        int *arr;
        int output[8];
        char c=a;
        for (i = 0; i <8 ; ++i) {
                 output[8-i-1] = (a >> i) & 1;

        }
        arr=&output[0];
//              for (i = 0; i <8 ; ++i) {
//              printf("%d", output[i]);
//      }
//      printf("\n");

        return arr;
}

The ouptut should be the binary value of the char 'a'which is: 0 1 1 0 0 0 0 1

but i got this instead: 0 -1216804320 -1218095335 -1216804320 10 -1076423592 -1218208721 -1216804320

Is this a pointer problem? How do i fix it so it would print the right answer? thx!!

2 Answers2

4

You're returning a pointer to a local variable (arr), whose contents are no longer valid when the function returns. Use malloc instead:

int main(void){
        ...
        int *arr = bin(a);
        ...
        free(arr);
        return 0;
}

int *bin(char a){
        int *arr = malloc(8 * sizeof int);
        ...
        return arr;
}
RichieHindle
  • 272,464
  • 47
  • 358
  • 399
  • You could pass the array into 'bin' also. No making sure of the matching free() being called. – Lee Meador Mar 18 '13 at 19:00
  • @user1887339 faith on what you understands – Grijesh Chauhan Mar 18 '13 at 19:57
  • 1
    @GrijeshChauhan: Did I say that? Now, for starters, this *is* a correct answer, contrary to the OP's claims. Furthermore, this answer *was* posted 45 Seconds before yours. – bitmask Mar 18 '13 at 20:13
  • @bitmask I misunderstood your comment that the reason Yes RichieHindle's answer was first. but I had long discussion with OP..there was problem of free also. But I don't think 45 second before is big difference. I like answers on content base...any No problem. – Grijesh Chauhan Mar 19 '13 at 02:39
1

First error I can notice is Scope problem, that you are returning address of local variable output[8];

 arr=&output[0];
 return arr;

That is wrong, scope of output[] is inside bin() function only.

You should allocate memory for output[] dynamically if you want to access it outside bin() function, like:

int *output = calloc(8, sizeof(int));

even you don't need extra variable just return output from bin()

I have corrected you code like below without much change:

int *bin(char a){
        int i=0;
        int *output = calloc(8, sizeof(int));
        for (i = 0; i <8 ; ++i) {
                 output[8-i-1] = (a >> i) & 1;
        }
        return output;
}

Notice removed unused variables c and arr

additionally, don't forgot to free dynamically allocated memory explicitly:

Second (Notice) be aware of Memory Clobbering Error because you are updating binary variable by ++ in printf function.

Just simple don't free(binary); it will produce run time error because you are modifying binary variable inside printf *binary++: first save return address in a extra pointer variable then free that. Like I am doing:

 b = binary=bin(a);      
 // loop 
  printf("%d", *binary++); 
 //             ^ changing binary variable to point new memory location  
 // after loop 
 free(b);

If you free(binary) then it would be wrong because you would free a memory location that is not allocated dynamically because your are changing binary variable.

get a working code here, read comments.

Grijesh Chauhan
  • 57,103
  • 20
  • 141
  • 208
  • problem solved! And do i need to free the allocated memories? – user1887339 Mar 18 '13 at 19:11
  • @user1887339 Yes you should free memory explicitly in your case it will not problem but its needed it your allocate memory again and again in lengthy code. see how do I free memory once binary becomes unused in code: at codepad – Grijesh Chauhan Mar 18 '13 at 19:14
  • @user1887339 Read todays [HOT question](http://stackoverflow.com/questions/15467298/how-far-can-memory-leaks-go) memory leak It help you to know why memory free required also read [wiki: Memory leak](http://en.wikipedia.org/wiki/Memory_leak) – Grijesh Chauhan Mar 18 '13 at 19:19
  • after i added free(binary);, it prints the following: – user1887339 Mar 18 '13 at 19:23
  • *** glibc detected *** ./a.out: free(): invalid pointer: 0x09b32028 *** ======= Backtrace: ========= and ======= Memory map: ======== – user1887339 Mar 18 '13 at 19:24
  • @user1887339 read updated answer my your are modifying `binary` variable so its pointing a new memory location. – Grijesh Chauhan Mar 18 '13 at 19:35
  • ok and if i'm adding another method hex(): int main(){ char a='a'; int *binary, *hexadecimal ,*b, *c; b=binary=bin(a); c=hexadecimal =hex(binary); free(b); free(c); } int *hex(int *binary){ //operations int *hexOutput = calloc(8, sizeof(int)); return hexOutput; } is the syntax correct, i need to create *c and add new calloc and free? – user1887339 Mar 18 '13 at 19:43
  • @user1887339 faith on what you understands. Read my updated answer there was other mistakes in your code that what `free(binary)` was producing error. Let me know if you need any more help on this. – Grijesh Chauhan Mar 18 '13 at 19:44
  • @user1887339 Yes that is correct as I understands. See Please understand. by `free(binary)` you were free a memory location that was not reserved dynamically because you are changing `binary` by `++` in printf statment. If you don't you not need other variable. – Grijesh Chauhan Mar 18 '13 at 19:48