There seems to be a segmentation when trying to print 'Cselection' after reading 'Pselection' error andd I dont understand why

Question

This is a program that takes input from user for his name and selection out of rock, paper and scissors. It has a function generateRandomNumber(int n) that generates 0, 1 or 2. based on the number the computer is assigned its selection and both their selections are compared to see who won the round. This repeats for 3 times and the one with the highest point wins. But there seems to be a segmentation error when trying to print computer's selection before comparison of the selections. I do not understand the reason behind this error. Please help me fix the code.

source code:

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

int generateRandomNumber(int n)
{
    srand(time(NULL));
    return rand() % n;
}

int main()
{
    int num;
    char PlayerName[50];
    char Pselection[8], Cselection[8];

    printf("Enter Name of Player:\n");
    fgets(PlayerName, sizeof(PlayerName), stdin);
    PlayerName[strcspn(PlayerName, "\n")] = '\0'; // Remove the newline character

    printf("Player 1: %s \nPlayer 2: Computer\n", PlayerName);

    int Cpoints = 0, Ppoints = 0;
    for (int i = 0; i < 3; i++)
    {
        printf("The Game of Rock, Paper, and Scissors begins:\n Enter your selection:\n");
        scanf("%s", Pselection);

        num = generateRandomNumber(3);
        if (num == 0)
            strcpy(Cselection, "rock");
        else if (num == 1)
            strcpy(Cselection, "paper");
        else if (num == 2)
            strcpy(Cselection, "scissors");
        printf("Compueter: %s\n", *Cselection);
        if (strcmp(Cselection, Pselection) == 0)
        {
            continue;
        }
        else if (strcmp(Cselection, "rock") == 0 && strcmp(Pselection, "paper") == 0)
            Ppoints++;
        else if (strcmp(Cselection, "paper") == 0 && strcmp(Pselection, "rock") == 0)
            Cpoints++;
        else if (strcmp(Cselection, "scissors") == 0 && strcmp(Pselection, "paper") == 0)
            Ppoints++;
        else if (strcmp(Cselection, "paper") == 0 && strcmp(Pselection, "scissors") == 0)
            Cpoints++;
        else if (strcmp(Cselection, "scissors") == 0 && strcmp(Pselection, "rock") == 0)
            Ppoints++;
        else if (strcmp(Cselection, "rock") == 0 && strcmp(Pselection, "scissors") == 0)
            Cpoints++;


        else
            printf("\t*******\tSelection Error! Please check your selection and try again.\t*******\t\n");
            
        
        printf("score: %d-%d", Ppoints, Cpoints);

    }

    if (Ppoints >= Cpoints)
        printf("The score is %d-%d \nCongratulations! %s is the winner!\n", Ppoints, Cpoints, PlayerName);
    else
        printf("The score is: %d-%d \nBetter Luck Next Time! The computer won.\n", Ppoints, Cpoints);

    return 0;
}

Error Message:

‘__builtin_memcpy’ writing 9 bytes into a region of size 8 overflows the destination [-Wstringop-overflow=]

I tried to print a few variations of printing the Cselection but the output stops after entering Pselection.

Answer 1

You had 2 mistakes.

1st Mistake:

printf("Compueter: %s\n", *Cselection);

You shouldn't have used '*'.

2nd Mistake:

char Pselection[8], Cselection[8];

You have as an input choice the word "scissors". To be able to store it to an array, you need 9 slots. You must consider the '\0' character as a part of it.

A code that will work:

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

int generateRandomNumber(int n){
    srand(time(NULL));

    return rand() % n;
}

int main(){
    int i;
    int num;
    int Cpoints, Ppoints;
    char PlayerName[50];
    char Pselection[8], Cselection[8];

    printf("Give a Name: ");
    fgets(PlayerName, sizeof(PlayerName), stdin);
    PlayerName[strcspn(PlayerName, "\n")] = '\0';

    printf("\nPlayer 1: %s\nPlayer 2: PC\n\n", PlayerName);

    Cpoints = 0;
    Ppoints = 0;
    puts("The Game of Rock, Paper, and Scissor begins.\nMay the Odds be in your Favor!\n");
    for (i=0; i<3; i++){
        puts("Make a Choice (rock, paper, scissor):");
        printf("%s: ", PlayerName);
        scanf("%s", Pselection);
    
        num = generateRandomNumber(3);
        if (num == 0)
            strcpy(Cselection, "rock");
        else if (num == 1)
            strcpy(Cselection, "paper");
        else if (num == 2)
            strcpy(Cselection, "scissor");
        
        printf("PC: %s\n", Cselection);
    
        if (strcmp(Cselection, Pselection) == 0);
        else if (strcmp(Cselection, "rock") == 0 && strcmp(Pselection, "paper") == 0) Ppoints++;
        else if (strcmp(Cselection, "paper") == 0 && strcmp(Pselection, "rock") == 0) Cpoints++;
        else if (strcmp(Cselection, "scissor") == 0 && strcmp(Pselection, "paper") == 0) Ppoints++;
        else if (strcmp(Cselection, "paper") == 0 && strcmp(Pselection, "scissors") == 0) Cpoints++;
        else if (strcmp(Cselection, "scissor") == 0 && strcmp(Pselection, "rock") == 0) Ppoints++;
        else if (strcmp(Cselection, "rock") == 0 && strcmp(Pselection, "scissors") == 0) Cpoints++;
        else printf("\t*******\tSelection Error! Please check your selection and try again.\t*******\t\n");
        
        printf("Score: %d/%d\n\n", Ppoints, Cpoints);
    }

    if (Ppoints >= Cpoints)
        puts("Congratulations! You Won!");
    else
        puts("Better Luck Next Time! PC Won!");

    return 0;
}

Answer 2

1. scanf("%s", Pselection); always use a maximum field width when reading a string with scanf() to avoid buffer overflows which in this case happens even with expected output "scissors". sizeof "scissors" == sizeof Pselection + 1.

   You can also use fgets() like you did earlier but you want to flush stdin if you didn't read a newline (which is expected for the input "scissors\n").

   int flush() {
      for(;;) {
        int c = getchar();
        if(c == EOF || c == '\n')
            return c;
      }
   }
   
    // ...
    char Pselection[sizeof "scissors"]
    char Cselection[sizeof "scissors"];
    // ...
        fgets(Pselection, sizeof Pselection, stdin);
        size_t j = strcspn(Pselection, "\n");
        if(j + 1 == sizeof Pselection) flush();
        Pselection[j] = '\0';
   

2. strcpy(Cselection, "scissors") is a buffer overflow like above.

3. Should Compueter be Computer?

4. printf("Compueter: %s\n", *Cselection); should be Cselection.

5. There is no reason to reset the seed on every random number. Movesrand(time(NULL)); from generateRandomNumber() to main().

6. Consider using string concatenation for multi-line output:

   printf(
          "Player 1: %s \n"
          "Player 2: Computer\n",
          PlayerName
   );
   

7. Consider mapping the user selection to an enum instead of mapping the computer selection to a string. For example:

   enum selection { ROCK, PAPER, SCISSORS };
   
   selection getSelection() {
        for(;;) {
           char Pselection[sizeof "scissors"];
           fgets(Pselection, sizeof Pselection, stdin);
           size_t j = strcspn(Pselection, "\n");
           if(j + 1 == sizeof Pselection) flush();
           Pselection[j] = '\0';
           if(!strcmp(Pselection, "rock"))
              return ROCK;
           if(!strcmp(Pselection, "paper"))
              return PAPER;
           if(!strcmp(Pselection, "scissors"))
              return SCISSORS;
           fprintf(stderr, "Selection Error!\n");
        }
   }
   

8. Consider using a rules table instead if all those conditions to figure out what the score for a given round. I would probably do it from the player's perspective instead of the computer:

      signed char rules[3][3] = {
            {0, -1, 1},
            {1, 0, -1},
            {-1, 1, 0}
      };
      signed char score = rules[getSelection()][generateRandomNumber(3)];
      if(score > 0)
          Ppoints+;
      else if(score < 0)
          Cpoints++;