CS50 - PSET3 Plurality - Segmentation fault

Question

#include <stdio.h>
#include <string.h>
// Max number of candidates
#define MAX 9

// Candidates have name and vote count
typedef struct
{
    string name;
    int votes;
}
candidate;

// Array of candidates
candidate candidates[MAX];

// Number of candidates
int candidate_count;

// Function prototypes
bool vote(string name);
void print_winner(void);

int main(int argc, string argv[])
{
    // Check for invalid usage
    if (argc < 2)
    {
        printf("Usage: plurality [candidate ...]\n");
        return 1;
    }

    // Populate array of candidates
    candidate_count = argc - 1;
    if (candidate_count > MAX)
    {
        printf("Maximum number of candidates is %i\n", MAX);
        return 2;
    }
    for (int i = 0; i < candidate_count; i++)
    {
        candidates[i].name = argv[i + 1];
        candidates[i].votes = 0;
    }

    int voter_count = get_int("Number of voters: ");

    // Loop over all voters
    for (int i = 0; i < voter_count; i++)
    {
        string name = get_string("Vote: ");
        // Check for invalid vote
        if (!vote(name))
        {
            printf("Invalid vote.\n");
        }
    }

    // Display winner of election
    print_winner();
}

// Update vote totals given a new vote
bool vote(string name)
{
    // TODO
    for (int i = 0; i <= candidate_count -1 ; i++)
    {
        if (strcmp(name, candidates[i].name) == 0)
        {
            candidates[i].votes = candidates[i].votes + 1;
            return true;
        }
    }
    return false;
}

// Print the winner (or winners) of the election
void print_winner(void)
{
    // TODO
    candidate temp[0];
    for (int i = 0; i < candidate_count; i++)
    {
        if((candidates[i].votes > candidates[i + 1].votes) && (i + 1 < candidate_count))
        {
            temp[0] = candidates[i];
            candidates[i] = candidates[i+1];
            candidates[i+1] = temp[0];
            i = -1;
        }

    }

for (int i = 0; i <= candidate_count; i++)
    {
        if (candidates[candidate_count - 1].votes == candidates[candidate_count - i].votes)
        {
            printf("%s\n",candidates[candidate_count - i].name);
        }
    }

}

Hello there, thank you for taking your precious time reading my question :)

I just got into CS50 and really appreciate the way David Malan is teaching. I have no real prior programming experience, though I had to program a little during school.

I looked for this answer in other threads, but did not find a corresponding one. Please feel free to rectify me if I have overseen a possible answer.

The problem now is the following: When I run this code, it works fine. It prints everything out as it should. Except that in most cases, after everything worked fine, it returns a segmentation fault (core dumped) after it gives the right output.

I assume that I refer to an array index that does not exist, but can't spot it. Debugging does not help either, as it shows the error at the very last curly bracket of main.

Thank you for your time :)

Edit: I added int temp2[candidate_count]; after sorting some things out. It does not do anything but declaring an int-array with the size of candidate_count. But with this, it works. I can't fathom the reason?

#include <stdio.h>
#include <string.h>
// Max number of candidates
#define MAX 9

// Candidates have name and vote count
typedef struct
{
    string name;
    int votes;
}
candidate;

// Array of candidates
candidate candidates[MAX];

// Number of candidates
int candidate_count;

// Function prototypes
bool vote(string name);
void print_winner(void);

int main(int argc, string argv[])
{
    // Check for invalid usage
    if (argc < 2)
    {
        printf("Usage: plurality [candidate ...]\n");
        return 1;
    }

    // Populate array of candidates
    candidate_count = argc - 1;
    if (candidate_count > MAX)
    {
        printf("Maximum number of candidates is %i\n", MAX);
        return 2;
    }
    for (int i = 0; i < candidate_count; i++)
    {
        candidates[i].name = argv[i + 1];
        candidates[i].votes = 0;
    }

    int voter_count = get_int("Number of voters: ");

    // Loop over all voters
    for (int i = 0; i < voter_count; i++)
    {
        string name = get_string("Vote: ");
        // Check for invalid vote
        if (!vote(name))
        {
            printf("Invalid vote.\n");
        }
    }

    // Display winner of election
    print_winner();
}

// Update vote totals given a new vote
bool vote(string name)
{
    // TODO
    for (int i = 0; i < candidate_count; i++)
    {
        if (strcmp(name, candidates[i].name) == 0)
        {
            candidates[i].votes = candidates[i].votes + 1;
            return true;
        }
    }
    return false;
}

// Print the winner (or winners) of the election
void print_winner(void)
{
    // TODO
 int temp2[candidate_count];


    candidate temp[0];
  for (int i = 0; i < candidate_count; i++)
    {
        if((candidates[i].votes > candidates[i+1].votes) && (i + 1 < candidate_count))
        {
            temp[0] = candidates[i];
            candidates[i] = candidates[i+1];
            candidates[i+1] = temp[0];
            i = -1;
        }

    }


for (int i = 0; i <= candidate_count; i++)
    {
       if (candidates[candidate_count - 1].votes == candidates[candidate_count - i].votes)
        {
            printf("%s\n",candidates[candidate_count - i].name);
        }
    }

}

If I take it out, it does not work properly anymore.

Answer 1

Your suspect line of code probably is:

    if((candidates[i].votes > candidates[i + 1].votes) && (i + 1 < candidate_count))

As this goes out of bounds past the end of your array of candidates. Just to check that out, I added in a quick test to peer into that region and print it.

if (i == candidate_count -1)
    printf("Suspect code ahead: name-> %s, votes: %d\n", candidates[i + 1].votes);
        if((candidates[i].votes > candidates[i + 1].votes) && (i + 1 < candidate_count))

When I ran the program on my machine with three candidates, this was the terminal output.

@Una:~/C_Programs/Console/Candidate/bin/Release$ ./Candidate A B C
Number of voters: 5
Vote: A
Vote: B
Vote: B
Vote: B
Vote: C
Suspect code ahead: name-> (null), votes: -1921036152
B

In my case, the program did not experience a segment fault, but could just as well have as it is reading out-of-bounds memory. What you probably need to do is some bounds checking so that you do not attempt to read past your array.

Check that out.