My own itoa (Help)

[bruntmjust]

Why is this code giving me a seg falut? How can I fix it?

Please help.

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

void good_itoa(int value, char* str, int base)
{
 static char dig[]="0123456789abcdef";

     int n=0, neg=0;
     unsigned int v;

     if(value<0)
      {
        value=-value;
        neg=1;
             }

     v=value;
     do
       {
          str[n++]=dig[v%10];
          v/=10;
          }while(v);

           if(neg)
           {
            str[n++];
              }
            str[n]='\0';


}


int main(){

    int value, base, l=0, n, r;
    char* str;

     printf("Enter your integer and its base\n");
     scanf("%d",value);
     printf("Now enter the base: \n");
     scanf("%d",base);
     n=value;

     while(n>1)
    {
      n=n/10;
      l+=1;
            }
     malloc((sizeof(char)*l)+1);
     good_itoa(value, str, base);
     printf("Your integer !");
     printf(str);
     printf("\n");
     free(str);
     return 0;

}

[Chervil]

scanf("%d", &value); note the &

str is a pointer but it is never given a valid address. The result of malloc is ignored).

Make sure that the buffer is large enough, looks like the malloc size is a bit too small.

[bruntmjust]

How exactly would I fix the str? and how do I know if my buffer is large enough and how to fix??

[Chervil]

Look at the example of malloc, notice the result is assigned to the buffer
http://www.cplusplus.com/reference/cstdlib/malloc/

How do you know if it is large enough? Well, your current method is almost right. It just needs tweaking. And remember to allow for the minus sign (if required).
It might be simpler to just use a fixed size, say 80 bytes, that should be adequate.

[bruntmjust]

got it! it works now for the most part, at least compile and print the string.

How would you make this a good itoa function out of this code?

[Chervil]

Well, ideally it should be able to handle any data that is fed to it, including bad data, such as a base which is too large.

I played around with that code a bit, but I certainly didn't make it bullet-proof.

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

void good_itoa(int value, char* str, int base)
{
    static char dig[]="0123456789ABCDEF";

    int n=0, neg=0;
    unsigned int v;

    if(value<0)
    {
        value=-value;
        neg=1;
    }

    v=value;
    do
    {
        str[n++]=dig[v%base];
        v /= base;
    } while(v);

    if (neg)
    {
        str[n++] = '-';
    }

    for (int i=0, m=n-1; i< m; i++, m--)
    {
        char temp = str[m];
        str[m] = str[i];
        str[i] = temp;
    }

    str[n]='\0';
}


int main(){

    int value = 123445;
    int base = 16;
    int l = 0;
    int n;
    int r;
    char* str = 0;

    printf("Enter your integer and its base\n");
    scanf("%d", &value);

    printf("Now enter the base: \n");
    scanf("%d", &base);
    n=abs(value);

    while (n>0)
    {
        n = n/base;
        l += 1;
    }
    str = (char *)  malloc((sizeof(char)*l)+2);
    good_itoa(value, str, base);

    printf("Your integer %s \n", str);
    free(str);
    return 0;

}

[bruntmjust]

Oh that is awesome! That helps a lot! How does the base part work? I see I am able to put in base 2 or 16 or even 10 and it works. Could you explain?

Thank you so much!

[Chervil]

There's no magic there. The original code used a fixed base of 10. I just replaced that with the variable base. When base is 10, the code is more or less doing the same as the original.

So, in order to understand it, you need to understand how it works for one base, such as 10, or maybe 2. When you understand that, the rest is just more of the same.

[bruntmjust]

Okay I see what you are saying, one last thing: What does the n=abs(value) do exactly for this function? I know it takes the absolute value of the value but why do this?

[Chervil]

Look at the context. The value input by the user may be negative or positive.
But at line 57 we have while (n>0)
If n is negative, the loop will never execute as it will fail this comparison. So I used that as a somewhat crude way of handling the situation. This isn't necessarily the best approach, as it doesn't mirror exactly the code inside the function good_itoa()

I look at it this way, right now we are interested in the internal workings of this function. But if you think of it as being like the library function (which exists for some compilers, but is not standardised), the user of such a function will treat it as a 'black box' which does something. In practice we would just pass a generously large buffer to the function, without going to the trouble of trying to pre-calculate the exact size.

My preferred approach would be to use a C++ std::string and let the function allocate this for itself, without the user having to be concerned with the size. Though the current code doesn't use any C++ features.

[bruntmjust]

Yes I see exactly what you are saying. I actually thing that is a really smart way of doing this. Thank you for all the help with the code and the extra explanation I really appreciate it.