Sum of Multiples: Segmentation fault

Programming C
1

Hi, i am getting a segmentation fault when i run my code.

My code:

#include "sum_of_multiples.h"

unsigned int sum(const unsigned int *factors, const size_t number_of_factors, const unsigned int limit)
{
    // finding the multiplies
    unsigned int multiplies[100];
    int index = 0;
    for(size_t i = 0; i < number_of_factors; i++)
    {
        unsigned int multiply = factors[i];
        for(int j = 1; multiply < limit; j++)
        {
            if(factors[i] == 0)
            {
                multiplies[index] = 0;
                index ++;
                break;
            }   
            multiply += factors[i] * j;
            multiplies[index] = multiply;
            index ++;
        }
    }
    // removing duplicates
    unsigned int duplicates_removed[100];
    int index_2 = 0;
    for(int k = 0; k < index + 1; k++)
    {
        bool is_duplicate = false;
        for(int l = 0; l < index + 1; l++)
        {
            if(multiplies[k] == multiplies[l])
                is_duplicate = true;
        }
        if(!is_duplicate)
        {
            duplicates_removed[index_2] = multiplies[k];
            index_2 ++; 
        }        
    }
    // summing the unique multiplies
    unsigned int energy_points = 0;
    for(int m = 0; m < index_2 + 1; m++)
    {
        energy_points += duplicates_removed[m];
    }
    return energy_points;    
}

Error:
image

2

I’m not a C expert, but you need to manage the memory yourself: you can’t just start using an array size 100, you need to allocate the memory for it, and you need to free that memory when you’re done with it.

Instead of

     unsigned int multiplies[100];

you probably want

    uint8_t multiplies = calloc(100, sizeof *multiplies);

    // the rest of the function

    free(multiplies);
    return energy_points;

See Dynamic memory management

You need to #include <stdint.h> for the uint8_t type.

1 Like
3

A segmentation fault happens when the program tries to access memory at an invalid location, typically one to which it does not have access.

multiplies has a length of 100. Are you sure that’s sufficient?

1 Like
4

I just analyzed the whole solution. There are multiple issues:

  • The length of the arrays is a problem. Take a look at the last test (test_solutions_using_include_exclude_must_extend_to_cardinality_greater_than_3). It calls sum() with 5 small factors and a limit of 10000. You will find many more numbers than 100 below that limit that are multiples of the factors.
    And the tests are not exhaustive, they are rather examples. You might want to follow @glennj’s advice and allocate memory dynamically instead of using two fixed-sized arrays.
  • The first loop is supposed to generate and store all multiples of the current factor. You might want to analyze the inner for loop closely, when it stops and how it updates multiply.
  • Take a closer look at the conditions of the last three for loops (with the loop variables k, l, and m). When exactly will they become false? Can you see the issue?
  • The second for loop (with the loop variable k) is supposed to copy unique multiples from multiplies to duplicates_removed, the inner for loop (with the loop variable l) should determine whether the current multiple should be copied. But this inner for loop does not do that. Examine it closely, can you find and fix the issues?
3 Likes
5

Thank you for the advice, now i understand the problem better.

6

Happy coding!