Segmentation fault at base converting in C

Question

trying to convert dec to 32-base, and then print it to a file.

const char digits[] = "0123456789ABCDEFGHIJKLMNOPQRSTUV";

    char* baseConverter(int num, int base)                                                          
    {   char* res;
        int i=0;
        if (num == 0 || base == 10)
        {
            snprintf(res,"%03x",num);
            return *res;    
        }
        while( num > 0 )
        {
            *(res+i) = digits[num%base];
            num=num/base;
        }
        return *res;
    }

and then at the output code :

sprintf(line, "%03s", baseConverter(i, 32);

but I keep getting that Segmentation fault (core dumped) error at running.

`res` is an uninitialised pointer that doesn't point anywhere sensible. Ouch! You must create an array of chars to fill. (And if you want to return it from the function, it must be an array that is dynamically allocated with `malloc`. It might be better to pass an array into the function.) — M Oehm, Mar 14 '16 at 19:24
(Side note: You don't increment `i` and you print the number backwards.) — M Oehm, Mar 14 '16 at 19:25
I've tried doing : char* res= (char*)malloc(4*sizeof(char)); (and increment for the i ) I'm still getting the segmentation error.. :/ — BVtp, Mar 14 '16 at 19:29
Do you have warnings enabled? Your `sprintf` call's arguments aren't correct. If you allocate only 4 bytes, you must enforce this limit. You must also return `res`, not `*res`. — M Oehm, Mar 14 '16 at 19:33
I tried replacing the line `... = digits[num%base]` with ` snprintf(res,"%s%c",*res,digits[num%base]);` and removing the `*` from return res. nothing works :/ — BVtp, Mar 14 '16 at 19:51
It's good that you use `snprintf`, but the second parameter must be the buffer size, in your case 4. You can't use the string you write to as argument after the format string, though. That's the cat biting its own tail, viz undefined behaviour. — M Oehm, Mar 14 '16 at 20:14

score 2 · Accepted Answer · answered Mar 14 '16 at 20:40

There are several things going on here:

First an uninitialised local pointer has an indeterminate value; it doesn't point anywhere in particular. The NULL pointer doesn't point anywhere either, but at least you can test for a NULL pointer easily. Make a habit of initalising a pointer to make it point to valid memory or to make it explicitly null.
The pointer is supposed to point to a char buffer. The way your function looks like, you must allocate memory for that buffer on the heap with malloc. (You can't use local storage, because that would be invalidated immediately.)
Don't make base 10 a special case. (You're even doing it wrong by printing base 10 numbers as hex.)
Your method of printing is okay, but you print the number backwards. So determine the required klength first and then decrement the position you print at.
Here, you deal with the raw characters. Use res[i] rather than do complicated things with the standard library functions. In particular, don't build strings by concatenating or printing strings to themselves. That's very likely undefined behaviour.

A possible implementation of your function could look like:

int ndigits(int num, int base)
{
    int n = 0;

    while (num) {
        n++;
        num /= base;
    }

    if (n == 0) n++;
    return n;
}

char* baseConverter(int num, int base)                                                          
{
    if (num >= 0 && base > 1 && base <= 36) {
        int n = ndigits(num, base);
        char *res = malloc(n + 1);
        int i = n;

        res[n] = '\0';
        if (num == 0) res[--i] = '0';

        while (num) {
            res[--i] = digits[num % base];
            num /= base;    
        }

        return res;
    }

    return NULL;
}

Note how an auxiliary function is used to determine the length of the string. The string is then filled backwards, staring with the null terminator. Also note how invalid cases are handled by returning NULL.

Your calling code must explicitly free the string after using it:

    int n = rand() % 100000 + 1;
    int m = rand() % 10 + 2;
    char *p = baseConverter(n, m);

    if (p) printf("%d#%d == %s\n", n, m, p);
    free(p);

C has manual memory management and keeping track of allocated stuff is tedious. You can't, for example, call baseConverter from inside printf, because you'd lose the handle to the allocated string.

Another popular variant is to have the calling code allocate the memory and then pas a buffer and its size to the function to fill it. A prototype could then look like this:

void sbase(char buf, size_t buflen, int num, int base);

It would then be called like this:

    char buf[33];     // Maximum, when base 2 is printed

    sbase(buf, sizeof(buf), 5000, 13);
    puts(buf);

Because buf is an automatic variable, no freeing is to be done. (How to implement thins and how to properly enforce that the buffer size isn't exceeded is left as an exercise. :))

anita2R · Answer 2 · 2016-03-15T12:07:51.483

The main errors have already been pointed out. Here is another suggested routine (it doesn't require malloc) The function sets the value of a pointer to the number of converted digits, to make it easy to print out the required digits.

#include <stdio.h>

/* function takes pointer to array, size of array + number/base 
   and pointer for number of digits in conversion */
void make32(int *res32, int len, int num, int base, int *rln);

int main()
{
  int digits32[20]; // size according to max conversion number in base 32
  int len32 = sizeof(digits32)/sizeof(digits32[0]);
  int in32, resln, n;

  /* convert this number */
  in32 = 10000;
  /* call function with pointer + size & number/base & ptr to # converted digits*/
  make32(digits32, len32, in32, 32, &resln);
  /* print out result - reverse order - use number of digits */
  for(n = resln; n >= 0;  n--) {
     printf("%d  ", digits32[n]);
  }
  printf("\n");
  return (0);
}

void make32(int *res32, int len, int num, int base, int *rln)
{
    int i = 0;
    while( num > 0 && i <= len ) {
        res32[i] = num % base;
        num = num / base;
        i++;
    }
    /* set the number of converted digits */
    *rln = i - 1;
}

Segmentation fault at base converting in C

2 Answers2