I am getting this Error: Segmentation fault (core dumped)

Question

This is a project that gets a string from user and prints how many vowels and constants the string has. The problem started when i created the fanctions malloc_memory and free_memory for more clear code so i can call the functions inside main and not allocating memory and free memory directly in main function. This is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define E_A_LETTERS 26
#define MAX_LENGTH 50

int check_vowels(char *p_string);
void malloc_memory(char **p_string);
void free_memory(char *p_string);

int main(void){
    // Here your code !
    char *string;
    int vowels;
    int constants;

    malloc_memory(&string);
    printf("Enter a string: ");
    fgets(string, MAX_LENGTH, stdin);

    vowels = check_vowels(string);
    constants = strlen(string) - vowels;

    printf("\nNumber of vowels : %d", vowels);
    printf("\nNumber of constants : %d\n", constants);

    free_memory(string);

}

int check_vowels(char *p_string)
{
    int i = 0;
    int count = 0;
    while(1)
    {
        if(*(p_string + i) == 'A' || *(p_string + i) == 'E' || *(p_string + i) == 'I' || *(p_string + i) == 'O' || *(p_string + i) == 'U')
            count++;
        if(*(p_string + i) == 'a' || *(p_string + i) == 'e' || *(p_string + i) == 'i' || *(p_string + i) == 'o' || *(p_string + i) == 'u')
            count ++;
        if(*(p_string + i) == '\0')
            break;
        i++;

    }
    return count;
}

void malloc_memory(char **p_string)
{
    p_string = (char **)malloc(MAX_LENGTH * sizeof(char) + 1);
    if(p_string == NULL)
    {
        printf("Unable to allocate memory...");
        exit(0);
    }
}

void free_memory(char *p_string)
{
    free(p_string);
}

And i am getting this output - error :

Enter a string: This is a string
Number of vowels : 4
Number of constants : 12
Segmentation fault (core dumped)

First thought - not related to the segmentation fault - is using numbers for characters makes the code super hard to read. 65 is `'A'`, 97 is `'a'`, etc. — Steve Friedl, May 22 '20 at 18:24
[Never use `gets()`](https://stackoverflow.com/q/1694036/10077)! — Fred Larson, May 22 '20 at 18:25
When you write "number of constants", do you maybe mean "number of **consonants**? — Roberto Caboni, May 22 '20 at 18:30
The construction `*(p_string + i)` is so common there is a more readable shortcut for it: `p_string[i]`. — Jongware, May 22 '20 at 18:32
This is not right: `p_string = (char **)malloc(MAX_LENGTH * sizeof(char) + 1);`. If you want to allocate a space for characters, it should be `char*` and not `char**` — Eugene Sh., May 22 '20 at 18:32
All of these comments are observations but none of these comments has a solution to the main purpose of posting it on stackoverflow — andreasm4, May 22 '20 at 18:33
You want to allocate to `*p_string`, so that `string` in `main`, where `pstring` points at, is updated. — M Oehm, May 22 '20 at 18:33
No, SO purpose is not (always) to give you a ready solution. Its purpose is to help *you* to come up with a solution. — Eugene Sh., May 22 '20 at 18:34
The way you've defined `malloc_memory`, `p_string` needs to be already defined, and it is since you are passing in `&string`. You want `*p_string = malloc(MAX_LENGTH * sizeof(char) + 1);`. (Don't cast the return value of `mailloc` either). Then check `if (*p_string == NULL) ...`. — lurker, May 22 '20 at 18:34
C11 draft standard n1570: *6.5.2.2 Function calls 4 An argument may be an expression of any complete object type. In preparing for the call to a function, the arguments are evaluated, and each parameter is assigned the value of the corresponding argument. 93) A function may change the values of its parameters, but these changes cannot affect the values of the arguments.* — EOF, May 22 '20 at 18:34
Comments are for asking for clarification of your code. That may include odd choices of wording, coding style, etc. *Answers* may appear in the larger space below. — Jongware, May 22 '20 at 18:36
@lurker so it should be: *p_string = (char *)malloc(MAX_LENGTH *sizeof(char) + 1); — andreasm4, May 22 '20 at 18:37
No, it should be exactly what I said it should be in my prior comment. — lurker, May 22 '20 at 18:39
Also, the number of consonants is incorrect. Just because a character is not a vowel does not make it a consonant. — Fred Larson, May 22 '20 at 18:42
@FredLarson I entered a sentence. It should be a word not a sentence. — andreasm4, May 22 '20 at 18:49

Vlad from Moscow · Answer 1 · 2020-05-22T18:50:59.483

3

The function malloc_memory is wrong.

Instead of these statements

p_string = (char **)malloc(MAX_LENGTH * sizeof(char) + 1);
if(p_string == NULL)

you have to write at least

*p_string = (char *)malloc(MAX_LENGTH * sizeof(char) );
if ( *p_string == NULL )

or

*p_string = malloc( MAX_LENGTH );
if ( *p_string == NULL )

After the call of fgets

fgets(string, MAX_LENGTH, stdin);

you should remove the possibly appended new line character '\n' to the entered string. For example

string[ strcspn( string, "\n" ) ] = '\0';

The function check_vowels could be written the following way

#include <ctype.h>

//...

size_t check_vowels( const char *p_string )
{
    const char *vowels = "AEIOU";
    size_t count = 0;

    for ( ; *p_string; ++p_string )
    {
        if ( strchr( vowels, toupper( ( unsigned char )*p_string ) ) != NULL )
        {
            ++count;
        }
    }    

    return count;
}

edited May 22 '20 at 18:50

answered May 22 '20 at 18:35

Vlad from Moscow

301,070
26
186
335

And `*p_string = malloc(...)` instead of `*p_string = (char *)malloc(...)`. – lurker May 22 '20 at 18:38
It worked. But i can't understand why it should be char * and not char ** – andreasm4 May 22 '20 at 18:38
@andreasm4 You passed the pointer string by reference that is through a pointer to it. Within the function you have to change the referenced pointer not the pointer that points to the pointer string. – Vlad from Moscow May 22 '20 at 18:40
1

@andreasm4 you are passing `&string` as a valid pointer to a pointer. So `p_string` (a `char **`) is already defined as the address of `string` and does not need to be allocated. `*p_string` is the `char *` you need to allocate. Since `p_string` is a `char **`, then `*p_string` is a `char *`. – lurker May 22 '20 at 18:41
@lurker I fully understand now what you told me. Thanks very much – andreasm4 May 22 '20 at 18:45

anastaciu · Answer 2 · 2020-05-23T15:35:38.120

As said your allocation is incorrect, it would be less complicated to return a pointer to the allocated space:

Function:

char* malloc_memory() {

    char* p_string = malloc(MAX_LENGTH); // a char is always 1 byte and no cast needed

    if(p_string == NULL) {
        printf("Unable to allocate memory...");
        exit(0); //or return NULL to handle it in the caller
    }
    return p_string;
}

Main:

char* string = malloc_memory();

On another note gets() is a dangerous function, vulnerable to overflow, you should use a method that limits the size of the string read from the buffer to the container size, something like:

scanf("%49[^\n]", string);

For a 50 char container. Reads until \n is found or 49 characters are read. So 49 characters plus the null terminator added by scanf.

I am getting this Error: Segmentation fault (core dumped)

2 Answers2