#include <stdio.h>

void DectoBin(int *n);
int *p;
int position;

int main()
{    
   int num;
   printf("Input number : ");
   scanf("%d", &num);
   DectoBin(&num);

   for (int i = position - 1; i >= 0; i--)
   {
       printf("%d", p[i]);
   }
}

when launch this code, this code compile well... but I have a error message 'zsh : segmentation fault'

void DectoBin(int *n)
{
    int binary[20] = { 0, };
    p = binary;

    while (1)
    {
        binary[position  ] = *n % 2;
        *n = *n / 2;
        if (n == 0)
            break;
    }

    return;
}
  

so, What parts should be corrected to solve the problem??

CodePudding user response：

For starters there is a typo

if (n == 0)

It seems you mean

if (*n == 0)

Though there is no any sense to except a number indirectly through a pointer to it.

You are using the local array binary with automatic storage duration within the function DectoBin

void DectoBin(int *n)

{
int binary[20] = {
    0,
};

//...

that will not be alive after exiting the function. So the pointer p will have an invalid value.

Also it is unclear why you are using the magic number 20 in the array declaration. At least you should use the value of the expression sizeof( int ) * CHAR_BIT.

Also it is a bad idea to use global variables.

At Least you could declare the array within the function with the storage-class specifier static and return a pointer to the array from the function.

Pay attention to that for negative numbers you can get an incorrect result.

For example the function can be implemented the following way as shown in the demonstration program below.

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

const char * DecToBin( int n )
{
    static char binary[CHAR_BIT * sizeof( int )   1 ];
    memset( binary, 0, sizeof( binary ) );

    unsigned int un = n;

    char *p = binary   sizeof( binary ) - 1;

    do
    {
        *--p = '0'   ( un & 1 );    
    } while ( un >>= 1 );

    return p;
}

int main( void )
{
    printf( "%d -> %s\n", 123, DecToBin( 123 ) );
    printf( "%d -> %s\n", -123, DecToBin( -123 ) );
}

The program output is

123 -> 1111011
-123 -> 11111111111111111111111110000101

CodePudding user response：

I think the minimum change to make it seem to work is this:

// you use *n as an integer, but test "n" 
// (which is its address, and will never be zero).
// So the while loop goes on indefinitely, eventually overflowing the buffer. 

if (*n == 0)
    break;

A more serious problem is that the buffer is allocated backwards: you have the pointer outside the function and the buffer is allocated on the stack inside the function. So, as soon as the function exits, the buffer is no longer "legal". The data is probably still there (on my system, it is) and you might even be able to use it as if nothing was amiss. On a short program, and depending on your system, you might not notice that the code has become a time bomb.

You ought to estimate how much of a buffer you need (how many binary digits), then allocate memory on the heap, using malloc(), check it worked, and pass that memory to the function - which will allocate nothing - together with the allocated size, so it can ensure it doesn't overflow the buffer.

The function would then return how many digits to actually print.

#include <stdio.h>
#include <malloc.h>

size_t DectoBin(size_t maxdigits, int *digits, int number);

int main() {
   int num;
   int *buffer;
   size_t maxdigits, position;

   printf("Input number : ");
   scanf("%d", &num);

   // Always check your inputs.
   if (num < 0) {
       printf("Negative numbers are not supported yet\n");
       return 1;
   }

   maxdigits = 1;

   {
       // Calculate how many digits are required. Could use ceiling of base-2 logarithm of num.
       int tmp = num;
       while (tmp > 0) {
           maxdigits   ;
           tmp /= 2;
       }
   }

   buffer = malloc(maxdigits * sizeof(int));
   if (NULL == buffer) {
       fprintf(stderr, "Out of memory\n");
       return 2;
   }

   position = DectoBin(maxdigits, buffer, num);

   for (size_t i = position; i > 0; i--) {
        printf("%d", buffer[i-1]);
   }
   printf("\n");
   return 0;
}

size_t DectoBin(size_t maxdigits, int *digits, int number) {
        size_t pos = 0;
        do {
                digits[pos  ] = number % 2;
                number /= 2;
        } while (number && pos < maxdigits);
        return pos;
}