c segmentation fault (core dumped) error

Question

I tried to compile this program but I got a message saying "segmentation fault (core dumped)", by the compiler. Could anyone please tell me what is wrong?

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

#define power(x,y) (int)pow((double)x,(double)y)

/*-------------------setBits-------------------*/
/* A function to set the i (and following) bit(s) to be 'number'. The function takes  'number' and adds the matching powers of 2 to 'destination'. */
void setBits(int number, int i, int *destination) {
      for( ; number!=0 ; number/=2 , i++)
        (*destination) += (number % 2) * (power(2, i));
}

/*-------------------getDigit-------------------*/
/* A function that returns a string of 'number' converted to base 32. */
char getDigit(int number) {
    char *digits = "0123456789ABCDEFGHIJKLMNOPQRSTUV";
    return digits[number];
}
/*-------------------get32-------------------*/
/* A function that returns a string of 'number' converted to base 32. */
char *get32(int number) {
    char *result = "";
    if (number/32 == 0)
        result[0] = getDigit(number);
    else strcat(result,get32(number/32));
    return result;
}

/*-------------------main-------------------*/
int main(){
    int test = 0;
    setBits(23, 5, &test);
    printf("%s", get32(test));
    return 0;
}

Also if anybody has tips to get the code better I'd love to get advices (: Thanks.

Answer 1

Handling strings in C is tricky; you cannot split and concatenate them easily as you can do in scripting languages. Strings are arrays of chars whose storage must be handled by the user. That's also the cause of your segmentation violation: You try to modify read-only memory.

Other answers suggest using malloc. This is a viable way to get memory to hold the strigs, but it has a drawback: The memory has to be freed. That means that you can't use the return value of get32 directly in printf, because you must keep the handle to the allocated string somewhere.

This also means that your recursive solution isn't suited to C, becaouse you'd have to free all intermediate strings.

In your case, the string you need is short. You want to print a 32-bit number in base 32. This number can have at most 7 digits, so you need eight bytes. (The eighth byte is for storing the terminating null character.)

Another useful method that doesn't use allocation is to pass in a buffer of a certain length and have the function fill it. The function must receive the array and its length as parameters:

char *get32(char buf[], int len, int number)
{
    const char *digits = "0123456789ABCDEFGHIJKLMNOPQRSTUV";
    int n = len;

    buf[--n] = '\0';
    memset(buf, '0', n);

    while (number) {
        buf[--n] = digits[number % 32];
        number /= 32;
    }

    return &buf[n];
}

This function writes a number with len - 1 digits, all zero initially, and then returns a pointer to the first non-zero digit, so that you can print it. The char buffer is provided by the calling function:

int main()
{
    int test = 0;
    char buf[8];

    setBits(23, 5, &test);
    puts(get32(buf, sizeof(buf), test));

    return 0;
}

Of course, if you want to print two numbers in the same printf, you must use different buffers, or the buffer will be overwritten.

Answer 2

You need to allocate memory for result. Add

#include <stdlib.h>

and change the declaration of result to

char *result = calloc( BUFSIZE, sizeof(char));

where BUFSIZE is the maximum length result can get plus one for the terminating '\0'. calloc() fills the allocated memory with zeroes so your result string will be terminated correctly.

To avoid a memory leak, you should then also free the buffer after each call to get32():

char *result = get32(...)
printf("%s", result);
free(result);

in main() and likewise after the call in get32() itself.

Answer 3

Your error lies here:

char *result = "";
strcat(result,get32(number/32));

man strcat will confirm you that the first expected argument is the destination where the resulting string is written. Since you gave result as the destination, strcat() will try to write to result. Unfortunately, this pointer points to a special destination of the binary program where its literal strings (here: "") are registered; this section is read-only.

strcat() tries to write to a read-only section => BOOM => SIGSEGV.

You should allocate (malloc()) memory for result and use strncat():

char* const result = malloc(SOME_SIZE*sizeof(char));
result[0] = '\0';
strncat(result, get32(number/32), SOME_SIZE);

And finally, don't forget to free(result) ;).