0

I have a program in which a user inputs how many sentences they want, and then then be prompted to input the sentences. they cannot enter more than ten sentences. when i test, if i enter more than 5, i get a bus error code. here is my code, i am calling two functions in .h files that i dont believe are relevant but i will provide them anyways.

//this program will take in user input for a number of sentences  up to ten, each with 100 char or less, and convert them all to uppercase

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "convert.h"


int main () {

int i; //increment
int numberofsentences; //number of sentences
char **sentence_array; // sentence array

sentence_array = (char **)malloc(numberofsentences*sizeof(char));

printf ("I will take up to ten sentences of up to 100 chars each, and convert them to uppercase, as well as give you stats on them. \n");
printf ("How many sentences would you like? \n");

scanf ("%d", &numberofsentences);
fgetc(stdin);


//user inputs the sentences based on the number that was provided
for (i = 0;  i< numberofsentences; i++) {
    sentence_array[i] = (char *)malloc(100 * sizeof(char));
    printf ("Enter sentence :");
    fgets(sentence_array[i], 101, stdin);
    printf ("\n");
      }

//call function that converts all arrays to uppercase
convertAll(sentence_array, numberofsentences);

printf("Your converted sentences are: \n");


//print out every sentence thats converted to uppercase
for (i = 0; i < numberofsentences; i++){
    //convertSentence(sentence_array[i]);
    printf("%s", sentence_array[i]);
       }
}

and functions.c file

#include <stdlib.h>
#include <ctype.h>
#include <stdio.h>
#include <string.h>
#include "convert.h"

//function to convert char array to uppercase
void convertSentence(char *sentence){
        int i;
        for (i = 0; i <strlen(sentence); i++){
                sentence[i] = toupper(sentence[i]);
        }
}

//function to convert array of all sentences and converts all to upper
void convertAll(char **sentenceList, int numOfSentences){
        int i;
        for (i = 0; i < numOfSentences; i++){
                convertSentence(sentenceList[i]);
        }
}

i feel like its something to do with the memory allocation. and as a note: i know that scanf and fgets are terrible, but my professor told us to use them so ... thanks for helping

jrdev
  • 125
  • 2
  • 13
  • `sentence_array = (char **)malloc(numberofsentences*sizeof(char));` => `sentence_array = malloc(numberofsentences*sizeof(char*));` or size is wrong – Jean-François Fabre Mar 13 '18 at 19:35
  • You are using `numberofsentences` variable for allocating memory before `scanf ("%d", &numberofsentences);` – H.S. Mar 13 '18 at 19:37

2 Answers2

1

that's expected because

sentence_array = (char **)malloc(numberofsentences*sizeof(char));

doesn't allocate enough memory for numberofsentences pointers. So after a few sentences stored, you get a memory violation or any other UB.

The sizeof is wrong, it should be:

sentence_array = malloc(numberofsentences*sizeof(char *));

Aside: no need for cast: Do I cast the result of malloc? (answer is no, BTW)

EDIT: my answer is incomplete, H.S. pointed out the other part of the problem (uninitialized value), that can be easily avoided by:

  • enabling the warnings
  • reading them

(plus the other fgets boundary error)

that'll teach me to try to valgrind OP code manually.

Jean-François Fabre
  • 137,073
  • 23
  • 153
  • 219
1

In this statement:

sentence_array = (char **)malloc(numberofsentences*sizeof(char));

you are using the local variable numberofsentences before initializing it, which is an undefined behavior. Also, sentence_array should be allocating memory of an array of char pointers i.e. numberofsentences*sizeof(char *) and move this statement below the numberofsentences input statement. So, it should be like this:

scanf ("%d", &numberofsentences);
fgetc(stdin);
sentence_array = malloc(numberofsentences*sizeof(char *));

[You don't need to cast the malloc result]

Also, here

fgets(sentence_array[i], 101, stdin);
                         ^^^

the buffer overflow can occur. The sentence_array[i] has been allocated memory to hold 100 characters and you are giving 101 as the maximum number of characters to be copied to sentence_array[i]. The fgets() reads characters from stream and stores them into the buffer (here, sentence_array[i]) until (num-1) (here, 101-1) characters have been read or either a newline or the end-of-file is reached, whichever happens first and a terminating null-character is automatically appended after the characters copied to the buffer. So, it could cause the buffer overflow. It should be:

fgets(sentence_array[i], 100, stdin);
H.S.
  • 11,654
  • 2
  • 15
  • 32