String garbage problem

[dev90]

I am getting garbage value. Is it need to free 's' every time or the reason is something else.
please,help.

#include<stdio.h>
#include<conio.h>
#include<alloc.h>
char* toBin(int);
void rec(int);
void main()
{
    int n;
    char *out;
    clrscr();
    printf("\n Enter no : ");
    scanf("%d",&n);

    out=toBin(n);
    printf("\n %s",out);
    free(out);
//  rec(5);
    getch();
}
/*void rec(int n)
{
    if(n<=0)
    {
        return;
    }
    rec(n-1);
    printf("\n %d",n);
}*/
char* toBin(int d)
{
    int d1=d,r=0,i=0;
    char *s="";
  //    free(s);
  //    s=(char*)malloc(10*sizeof(char));
    while(d1>0)
    {
        r = d1 % 2;
        d1 = d1 / 2;
        s[i] = r + 48;
        i++;
    }
    flushall();
    strrev(s);
    flushall();
    printf("\n rev : %s",s);
    return s;
}

/*output

Enter No : 33
% : 100001
 s% : 33

 */

[dev90]

This program is of decimal to binary conversion.

[Moschops]

No. Only free something you allocated using malloc. You don't use malloc, so don't use free.

The problems.

Firstly, you are returning a pointer from your function toBin. What does it point at? A local variable, made inside that function. The local variable shouldn't exist anymore once the function ends. As it is, you're actually pointing at a piece of read-only memory because you assigned it using a string literal (anything you code like this,"some letters", is a string literal, and you are not allowed to write to that memory), so it does still exist, but that's by chance and not because you did it deliberately. However, there is a major problem; you're trying to write over it.

See this? s[i] = r + 48; You're not allowed to write over a string literal. Even if you were allowed to, how big is the string? It is of size zero. There are no letters in it.. So what are you writing over? Some other memory. This is a complete disaster.

If you want to create an array of char, do so. Pick a size, and allocate it. Then, if you want to write over that array, make sure you only write inside the array. Do not make an array like this char array[10] and then expect to be able to write over bits that don't exist, like this array[243] = 'x';, for example.

[dev90]

Thank you, Moschops.

I have 2 doubts. I have tried this:
(1)

char *s;
s=(char*)malloc(100*sizeof(char));

But as i am returning a pointer s I can't free it before returning it.

(2) And Second, I take a char array[100],Perform operation on it,and then point my pointer to it at the end.

1)char array[100];
2) while(d1>0)
    {
        r = d1 % 2;
        d1 = d1 / 2;
        array[i] = r + 48;
        i++;
    }
3)s=array;
4)strrev(s);

But in both the cases i am getting garbage values.Where I am wrong now?

[deceptikon]

But as i am returning a pointer s I can't free it before returning it.

The caller has to free it. While the ideal for robustness is to have memory allocated and freed at the same execution level, that's not always practical.

I take a char array[100],Perform operation on it,and then point my pointer to it at the end.

In this case you're returning a pointer to a local object. The local object gets destroyed when the function returns, so the caller gets what's called a dangling pointer--it points to garbage by definition.

[dev90]

then what should i do....? I am confused.
Because I have bees asked to return char* only.

[deceptikon]

then what should i do....?

Call malloc in toBin and free the returned pointer in main.

[dev90]

Sorry, but in TC still garbage is coming.
I have used malloc in toBin to s.
And free(out) in main.

[deceptikon]

Post your current code.

[dev90]

This is the one way.
Inline Code Example Here

#include<stdio.h>
#include<conio.h>
#include<alloc.h>
char* toBin(int);
void main()
{
    int n;
    char *out;
 // out=(char*)malloc(200*sizeof(char));
    clrscr();
    printf("\n Enter no : ");
    scanf("%d",&n);

    out=toBin(n);
    printf("\n OUT %s",out);
    free(out);
    getch();
}
char* toBin(int d)
{
    int d1=d,r=0,i=0;
 // char *s;
    char a[200];
 // s=(char*)malloc(200*sizeof(char));

    while(d1>0)
    {
        r=d1%2;
        d1=d1/2;
        a[i]=r+48;
        i++;
    }
    //strrev(a); forget this step,as it is not an issue.
    printf("\n rev  ptr %s",a);
    return a;
}

//This is another way using pointer.

Inline Code Example Here

char* toBin(int d)
{
    int d1=d,r=0,i=0;
    char *s;
    char a[200];

    s=(char*)malloc(200*sizeof(char));

    while(d1>0)
    {
        r=d1%2;
        d1=d1/2;
        a[i]=r+48;
        i++;
    }
    s=a;
    strrev(s);
    printf("\n rev  ptr %s",s);
    return s;
}

[deceptikon]

Compare and contrast:

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

char* toBin(int);

int main(void)
{
    int n;
    char *out;

    printf("\n Enter no : ");
    scanf("%d", &n);

    out = toBin(n);
    printf("\n OUT %s\n", out);
    free(out);

    return 0;
}

char* toBin(int d)
{
    int d1 = d, r = 0, i = 0;
    char *s;

    s = (char*)malloc(200 * sizeof(char));

    while (d1 > 0) {
        r = d1 % 2;
        d1 = d1 / 2;
        s[i] = r + 48;
        i++;
    }

    s[i] = '\0'; /* Always close the string! */
    _strrev(s);
    printf("\n rev  ptr %s", s);

    return s;
}

[dev90]

Thank you.

Got the problem. :)