2

I am implementing a program to divide all value in a array by 100 then store them in b array using malloc. The problem is I got segmentation fault when printing value of b in main.

This is my code

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

void divide(int *a, int n, double *b){
    b=malloc(n*sizeof(double));
    
    for(int i=0; i<n; i++){
        b[i]=(double)a[i]/100.0;
    }

    //check: values still remain in b
    for (size_t i = 0; i < 5; i++)
    {
        printf("%.2f ", b[i]);
    }
}

int main(){
    int a[]={1,2,3,4,5};
    double *b;

    divide(a,5,b);
    
    //check: lost value and cause segmentation fault
    for (size_t i = 0; i < 5; i++)
    {
        printf("%.2f ", b[i]);
    }
    free(b);
    return 0;
}

So what cause this problem and how to fix it?

Thanks in advance.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
Becker
  • 147
  • 7
  • 1
    `b=malloc(n*sizeof(double));` means nothing to the caller of `divide`. The `b` in main remains unchanged. , and in the process you leak the memory for the local `b` allocation in `divide` as well. Either pass the `b` argument by address (so a pointer to pointer) or utilize the otherwise unused return result of your function. Thiis is a *very* common C-beginner problem, and there are *hundreds* of duplicates to this question, but alas the vernacular and problem descriptions are so diverse it makes them difficult to search for. If I find a link I'll post it. – WhozCraig Dec 18 '21 at 09:25
  • @WhozCraig I thought value from `malloc` family will remain until the program terminate? – Becker Dec 18 '21 at 09:27
  • 1
    `malloc` is not relevant to the core issue. The core issue is that the assignment to a parameter inside a function does not modify the argument in the calling function. The argument `b` in the calling function and the parameter `b` in the function `divide` are different objects. – M. Nejat Aydin Dec 18 '21 at 09:30
  • @Becker That isn't the problem. the problem is the assignment. `b` in `divide` is literally a local variable to that function, and in C arguments are pass by *value*. If you want to modify something on the caller side from `main`, in this case a pointer `b`, then you need to either pass *that* by address and use dereference semantics in `divide`, or use the function result o convey the resulting allocation. Vlad's answer implements the second of these options. – WhozCraig Dec 18 '21 at 09:30
  • @WhozCraig there is not a real memory leak in this case, the system will regain that space sooner or later, reading an uninitialized pointer is the problem here. – David Ranieri Dec 18 '21 at 09:30
  • 1
    @DavidRanieri Regarding memory management, that is utterly irrelevant. One can say the same about *any* program ,riddled with memory leaks or otherwise, that isn't intended to run ad-infinitum (e.g. a perpetual service process). The habit is dreadful, and should be quelled as early in a young engineers learning path as plausible. – WhozCraig Dec 18 '21 at 09:35
  • 1
    @WhozCraig I agree, but young engineeers should also distinguish between what is a "real" memory leak and what is not: https://stackoverflow.com/a/274433/1606345 the OP snippet doesn't leak memory. – David Ranieri Dec 18 '21 at 09:45
  • 2
    @DavidRanieri Memory is allocated every time the function `divide` is called and that memory cannot be accessed once the function returns, that is simply lost. In my eyes, it is truly a memory leak. – M. Nejat Aydin Dec 18 '21 at 10:29

2 Answers2

3

You are passing the pointer b by value to the function divide

divide(a,5,b);

That is the function deals with a copy of the original pointer. Changing the copy does not influence on the original pointer.

You need either to pass the pointer by reference through a pointer to it or redesign the function such a way that it will return a pointer to the dynamically allocated memory within the function.

For example the function could be declared and defined the following way

double * divide( const int *a, size_t n )
{
    double *b = malloc( n * sizeof( double ) );

    if ( b != NULL )
    {
        for ( size_t i = 0; i < n; i++ )
        {
            b[i] = a[i] / 100.0;
        }

        //check: values still remain in b
        for ( size_t i = 0; i < n; i++ )
        {
            printf("%.2f ", b[i]);
        }
    }

    return b;
}

And in main you can write

double *b = divide( a, sizeof( a ) / sizeof( *a ) );

Otherwise the function can look like

void divide( const int *a, size_t n, double **b )
{
    *b = malloc( n * sizeof( double ) );

    if ( *b != NULL )
    {
        for ( size_t i = 0; i < n; i++ )
        {
            ( *b )[i] = a[i] / 100.0;
        }

        //check: values still remain in b
        for ( size_t i = 0; i < n; i++ )
        {
            printf("%.2f ", ( *b )[i]);
        }
    }
}

And called like

divide( a, sizeof( a ) / sizeof( *a ), &b );
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
2

What the function divide receives is a copy of the pointer b. This means that the variable b in function main is unchanged after the call. A simpler example to illustrate this is

void f(int n)
{
   n = 1;
}

After f has been called, n is unchanged on the caller's side.

The simplest correction of your code is to make divide receive a pointer to the pointer b. The signature of divide would then be

void divide(int *a, int n, double **b);

However, it's better to allocate and free memory in the same function since it's easy to forget that divide allocates memory. When working with arrays I always use two convenient macro functions NEW_ARRAY and LEN which simplifies the code. Below is my suggestion. If the length of a is changed the rest of the code will follow nicely.

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

#define LEN(array) (sizeof (array) / sizeof (array)[0])

#define NEW_ARRAY(pointer, length) \
    { \
        (pointer) = malloc(((size_t) length) * sizeof (pointer)[0]); \
        if ((pointer) == NULL) { \
            fprintf(stderr, "Allocating memory with malloc failed: %s\n", strerror(errno)); \
            exit(EXIT_FAILURE); \
        } \
    }

void divide(int *a, int n, double *b)
{
    for (int i = 0; i < n; i++) {
        b[i] = a[i] / 100.0;
    }
}


int main(void)
{
    int a[] = {1, 2, 3, 4, 5};
    double *b;

    NEW_ARRAY(b, LEN(a));
    divide(a, LEN(a), b);
    for (size_t i = 0; i < LEN(a); i++) {
        printf("%.2f ", b[i]);
    }
    printf("\n");
    free(b);

    return 0;
}
August Karlstrom
  • 10,773
  • 7
  • 38
  • 60