0

I'm a beginner programmer, I got a home assignment to seperate a string into words, and put each word in an array of strings. We are practicing dynamic memory allocations. the assignment says that the size of the array must be [10] and i need to change the size of the array with malloc accoring to the number of words in the string, and allocate room for every word in the array. when i reach the end of the programm and free the allocated memory it says "Project.exe has triggered a breakpoint" and i can't find my mistake in the code.

P.S this is my first question on stack so i apologize in advance if I posted wrong somehow.

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void fillArray(char string[], char* array[], int* pointer);
int countCharacters(char string[], int index, int* pointer);
void freeArray(char* arr[], int size);


void main()
{
    char string[] = { "i have two dreams" };
    printf("Your sentence is: %s", string);
    int sentenceLength = 1;
    for (int i = 0; string[i] != '\0'; i++)
    {
        if (string[i] == ' ') sentenceLength++;
    }
    int* point = &sentenceLength;
    char* array[10];
    fillArray(string, array, point);
    printf("\n\nYour array is: \n");
    for (int i = 0; i < *point; i++) puts(array[i]);
        freeArray(array, *point);
}


void fillArray(char string[], char* array[], int* pointer)
{
    *array = (char*)malloc(*pointer * sizeof(char));
    if (array == NULL)
    {
        printf("--NO MEMORY--");
        exit(1);
    }
    int i = 0;
    int j = 0;
    for (i; i < *pointer; i++)
    {
        array[i] = (char*)malloc(sizeof(char) * countCharacters(string, j, pointer));
        if (array[i] == NULL)
        {
            printf("--NO MEMORY--");
            exit(1);
        }
        for (j; string[j] != ' '; j++)
        {
            if (string[j] == '\0')
            {
                array[i][j] = '\0';
                return;
            }
            array[i][j] = string[j];
        }
        if (string[j] == ' ' || string[j] == '\0')
        {
            array[i][j] = '\0';
            j++;
        }
    }
}


int countCharacters(char string[], int index, int* pointer)
{
    int size = 1;
    if (string[index] == ' '&& index<= *pointer) index++;
    for (index; string[index] !=' '&& string[index]!='\0'; index++)
    {
        size++;
    }
    return size;
}


void freeArray(char* arr[], int size)
{
    for (int i = 0; i < size; i++)
    {
        free(arr[i]);
        arr[i] = NULL;
    }
}
ryyker
  • 22,849
  • 3
  • 43
  • 87
  • Why do you allocate memory for the pointer array when you already have `char *array[10]` in `main()`, which was passed to the function? – Weather Vane Jan 11 '21 at 13:28
  • Your `array` is not dynamically allocated, so you cannot free it. The pointers it contains to dynamically allocated space, on the other hand, must each be freed individually. Each `malloc()` call must be paired with exactly one corresponding `free()` call. – John Bollinger Jan 11 '21 at 13:28
  • I `Project.exe has triggered a breakpoint` the exact wording of the error message? – ryyker Jan 11 '21 at 13:32
  • @WeatherVane The assignment says that at the end of the programm the size of the array must be the number of words in the sentence – AlexLevitin Jan 11 '21 at 13:35
  • @ryyker Project.exe has triggered a breakpoint is the error i get in the debugger when i reach the free function, if i run it noramlly the output is fine and doesnt say any warning – AlexLevitin Jan 11 '21 at 13:36
  • In that case define `char **array` in `main()` and in the function and `return` that to `main()`, instead of passing it. – Weather Vane Jan 11 '21 at 13:36
  • Using `exit(1);` is not good for memory failures, instead use `break` and free the previous allocated memory, then return from `main` – IrAM Jan 11 '21 at 13:38
  • Settings for debug mode in my environments include an option for _breaking on library errors_ that can be turned on or off. This might be similar for yours, and is likely what you are seeing. It appears that because you are attempting to `free()` an object that was not allocated using dynamic memory, it is letting you know it cannot be freed. (I think this issue has also been addressed in previous comments, so probably no need to say anything more about it.) – ryyker Jan 11 '21 at 13:42
  • isn't `*array = (char*)malloc(*pointer * sizeof(char));` allocating memory to the array? – AlexLevitin Jan 11 '21 at 13:45
  • In fillArray, you initialize j to 0 and then increment it and use it for 2 different uses at the same time (array[i][j] = string[j]) : after first word array[i][j] is outside allocated array[i] – Ptit Xav Jan 11 '21 at 13:46
  • Yes, but not in the way you expect. See the second comment by John Bollinger. – ryyker Jan 11 '21 at 13:56
  • @ryyker so what is the correct way to allocate memory to the array? i thought that `*array = (char*)malloc(*pointer * sizeof(char));` gets the job done. – AlexLevitin Jan 11 '21 at 14:06
  • In both of your `malloc()` statements the expression `sizeof(char)` is _always_ equivalent to `1`, so completely unnecessary. And in C (however not C++) [casting the return of `malloc` (or `calloc` and `realloc`0 is wrong](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). The two statements should be: `*array = malloc(*pointer);` and `array[i] = malloc(countCharacters(string, j, pointer));`. And consider what happens if `countCharacters` fails. It should be called prior to the `calloc` call, and only its results should be passed as an argument. – ryyker Jan 11 '21 at 14:06
  • This statement: char* array[10]; creates an array of 10 pointers to char. It is the pointer element of the object that you are dynamically allocating, not the array element of the object. If this is what you want, it makes sense to allocate memory for each of the 10 pointers, not just one of them. (Or at least one time per word in your sentence, each corresponding to the length of word (for up to 10 sentences.) – ryyker Jan 11 '21 at 15:44
  • @AlexLevitin - See answer below addressing memory allocation and freeing issues. – ryyker Jan 11 '21 at 18:00

1 Answers1

0

"the assignment says that the size of the array must be [10] and i need to change the size of the array with malloc accoring to the number of words in the string" ... "it says "Project.exe has triggered a breakpoint" and i can't find my mistake in the code."

There may be other issues in your code, but based on comments, the largest question you expressed has been the problem of allocating and freeing memory. The following addresses those points only.

This statement:

char* array[10];  

Creates an array of 10 pointers to char. In the context of your assignment parameter stated above this limits you to a maximum of 10 words in any given sentence. This fact appears to be in conflict with the assumption made in your statement need to change the size of the array with malloc according to the number of words. The dynamic allocation of each pointer will set the number of characters. (not number of words.). Because there are 10 pointers, there can be up to 10 calls to malloc(), 1 for each pointer that you have created. But in your design, you will only make the number of call as there are words in your sentence, each allowing enough memory to accommodate each word

For example, "i have two dreams" has 4 words, requiring 2, 5, 4 and 7 bytes of memory respectively (accounting for an extra byte for null termination.) This will require 4 calls to malloc, each with a unique length value.

(Assuming _length_values below are being returned from your call to countCharacters(string, j, pointer) include the terminating null, the following will use: length

Note that in your code, this first call to malloc is not necessary:

*array = (char*)malloc(*pointer * sizeof(char));

And in the loop ( for (i; i < *pointer; i++) ) the call array[i] = (char*)malloc(sizeof(char) * countCharacters(string, j, pointer)); should be broken up into 2 separate calls to mitigate failed calls to countCharacters:

in length = countCharacters(string, j, pointer);
if (length > 0)
{
     array[i] = malloc(length);//cast and sizeof(char) are removed
     ...   

Based on your selected sentence: "i have two dreams", there should be 4 calls, each containing a value for length corresponding to your precending call to countCharacters

array[0] = malloc(length); //2
array[1] = malloc(length); //5
array[2] = malloc(length); //4
array[3] = malloc(length); //7

This will create memory that can be expressed as:

enter image description here

Correspondingly, each one explicit call to free needs to occur for each previously made call to malloc

Based on above, there should be 4 calls in this segment, but indexed accordingly:

for (int i = 0; i < *point; i++) puts(array[i])
                                               ^ removed ;
        freeArray(array[i]);
                       ^^^

Using a modified version of freeArray

   void freeArray(char* arr[])
   {
        free(arr[i]);
        arr[i] = NULL;
   }
ryyker
  • 22,849
  • 3
  • 43
  • 87