segmantation fault in c code

Question

I have a problem with a code I'm checking. I get a Segmentation fault (core dumped) and I think the problem is in this part of the code

The code supposed to add a new item to a connected list by users input.

The input should look like this

word_#_1999_#_synonym_#_FORIGEN

thank you in advance

// Add a new word to the dictionary with the format of { devoted_#_2003,_2001,_2008_#_worship_#_AHAVA }
struct Word * addWord(struct Word * dictionary)
{
    struct Word *scan = dictionary;
    struct Word *newWord = (struct Word *)malloc(sizeof(struct Word));
    char *input = (char *)malloc(sizeof(char) * 128);
    char *inputBackup = (char *)malloc(sizeof(char) * 128);
    char *years = (char *)malloc(sizeof(char) * 128);
    int count = 0;
    int tempYear;
    char *wordOfNewWord;

    printf("Please enter a new word into dictionary\n");
    scanf("%s", input);
    strcpy(inputBackup, input);

    // Init newWord
    newWord = (struct Word *)malloc(sizeof(struct Word));
    newWord->next = NULL;
    newWord->numOfYears = 0;

    // Check if good
    if(countSubstring(input, "_#_") != 3)
    {
        printf("error\n");
        return NULL;
    }

    // Get the word name
    wordOfNewWord = strtok(input, "_#_");
    newWord->word = (char *)malloc(sizeof(wordOfNewWord));
    strcpy(newWord->word, wordOfNewWord);

    // Get the word years
    years = strtok(NULL, "#");
    newWord->numOfYears = countSubstring(years, ",_") + 1;
    newWord->years = (unsigned short *)malloc(sizeof(unsigned short) * newWord->numOfYears);

    years = strtok(years, ",_");
    tempYear = strtol(years, NULL, 10);

    if (tempYear <= 9999 && tempYear > 0)
    {
        *(newWord->years + count) = tempYear;
    }
    else
    {
        printf("error\n");
        return NULL;
    }

    count = 1;
    years = strtok(NULL, ",_");
    while (years != NULL)
    {
        tempYear = strtol(years, NULL, 10);

        if (tempYear <= 9999 && tempYear > 0)
        {
            *(newWord->years + count) = tempYear;
        }
        else
        {
            printf("error\n");
            return NULL;
        }
        count++;
        years = strtok(NULL, ",_");
    }

    // Get word synonims
    strcpy(input, inputBackup);
    input = strtok(input, "#");
    input = strtok(NULL, "#");
    input = strtok(NULL, "#");
    newWord->numOfSynonym = countSubstring(input, ",_") + 1;
    newWord->synonymWords = (char **)malloc(sizeof(char) * 30 * newWord->numOfSynonym);

    input = strtok(input, ",_");
    *(newWord->synonymWords) = input;

    count = 1;
    input = strtok(NULL, ",_");
    while (input != NULL)
    {
        *(newWord->synonymWords + count) = input;
        count++;
        input = strtok(NULL, ",_");
    }

    // Get translation
    input = (char *)malloc(sizeof(char) * 120);
    strcpy(input, inputBackup);
    input = strtok(input, "#");
    input = strtok(NULL, "#");
    input = strtok(NULL, "#");
    input = strtok(NULL, "#");
    newWord->numOfTrans = countSubstring(input, ",_") + 1;
    newWord->tranWords = (char **)malloc(sizeof(char) * 30 * newWord->numOfTrans);

    input = strtok(input, ",_");
    *(newWord->tranWords) = input;

    count = 1;
    input = strtok(NULL, ",_");
    while (input != NULL)
    {
        *(newWord->tranWords + count) = input;
        count++;
        input = strtok(NULL, ",_");
    }

    // Put the new word in the dictionary
    if(findWord(dictionary, newWord->word) == 1)
    {
        printf("error\n");
        return NULL;
    }
}

there is the struct

struct Word
{
    char *word;
    unsigned short * years;
    unsigned short numOfYears;
    char ** synonymWords;
    unsigned short numOfSynonym;
    char ** tranWords;
    unsigned short numOfTrans;
    struct Word *next;
};

debugger should help you. Hope you know the possible reason for segmentation fault.Search it on google.That would be enough to find it. — Rafed Nole, Jan 06 '14 at 12:29
Again: _don't cast pointers returned by `malloc` when writing C_, a void pointer is all you'll ever need — Elias Van Ootegem, Jan 06 '14 at 12:32
No harm in doing it either, a better documentation practice maybe ? — Rafed Nole, Jan 06 '14 at 12:33
@RafedNole: It _can_ hide errors (missing `#include`) + best documentation, IMO is `malloc(sizeof(*var));` but that might be a personal thing — Elias Van Ootegem, Jan 06 '14 at 12:34
@EliasVanOotegem when using `sizeof` on a variable you can even omit the parentheses: `malloc(sizeof *var)` — Kninnug, Jan 06 '14 at 12:38
@RafedNole, it is definitley better to use the syntax that Elias uses. I once wasted more than a week on finding a bug in production code on exactly this problem. — Devolus, Jan 06 '14 at 12:39
@RafedNole: As an added bonus to using `sizeof *var`, if ever you decide to change the type, say from ` int *` to `unsigned long long int` or something, all your `realloc` and `malloc` calls will still work just fine, without having to change every `sizeof(int)` to `sizeof(new_type)` — Elias Van Ootegem, Jan 06 '14 at 12:41

Elias Van Ootegem · Answer 1 · 2014-01-06T12:49:27.917

For a kickoff, this code doesn't make much sense:

if (tempYear <= 9999 && tempYear > 0)
{
    *(newWord->years + count) = tempYear;
}

considering the only time count is used prior to this line is: int count = 0;

Other than that, you seem to be oblivious to the fact that char ** and char * are not the same thing!:

newWord->synonymWords = (char **)malloc(sizeof(char) * 30 * newWord->numOfSynonym);
//and
newWord->tranWords = (char **)malloc(sizeof(char) * 30 * newWord->numOfTrans);

Which are allocating char *, but at the same time, you're doing:

char *wordOfNewWord;
newWord->word = (char *)malloc(sizeof(wordOfNewWord));

Which is actually allocating memory to hold a pointer, depending on the architecture (32 or 64 bits) a pointer generally requires 4 to 8 times as much memory as a char, of which the size is by definition 1.
Please allocate memory according to the suggestions made in the comments:

char **pointers_to_strings = malloc(10*sizeof(*pointers_to_strings));
for(int i=0;i<10;++i)
    pointers_to_strings[i] = calloc(101, sizeof(*pointers_to_strings[i]));

To ensure you're always allocating the right amount of memory, required to accomodate whatever type it will hold.
And of course, no malloc or calloc without a free call:

for (int i=0;i<10;++i)
{
    free(pointers_to_strings[i]);
    pointers_to_strings[i] = NULL;
}
free(pointers_to_strings);
pointers_to_strings = NULL;

score 1 · Answer 2 · answered Jan 06 '14 at 12:35

1

the problem is in this lines

char *wordOfNewWord;
newWord->word = (char *)malloc(sizeof(wordOfNewWord));
strcpy(newWord->word, wordOfNewWord);

answered Jan 06 '14 at 12:35

Peter Miehle

5,984
2
38
55

Can you plase articulate the problem with these lines? – Notinlist Jan 06 '14 at 12:58
sizeof(wordOfNewWord) => sizeof(char*) => 4 or 8 => too small for strcpy – Peter Miehle Jan 06 '14 at 13:00

segmantation fault in c code

2 Answers2