C - Valgrind "Invalid read/write of size 1" errors while strcpy/strlen

Question

I have problem with seg.faults. The program works good, but for few unknown strings it results in segmentation fault. I ran the program with Valgrind and it reported "Invalid read/write of size 1", mostly the problem is connected with strcpy and strlen.

==5623== ERROR SUMMARY: 12 errors from 4 contexts (suppressed: 2 from 2)
==5623== 
==5623== 1 errors in context 1 of 4:
==5623== Invalid write of size 1
==5623==    at 0x4C2D812: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x400955: sameWords (main.c:62)
==5623==    by 0x400A6F: main (main.c:85)
==5623==  Address 0x51fd093 is 0 bytes after a block of size 3 alloc'd
==5623==    at 0x4C2CD7B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x40092B: sameWords (main.c:59)
==5623==    by 0x400A6F: main (main.c:85)
==5623== 
==5623== 
==5623== 1 errors in context 2 of 4:
==5623== Invalid write of size 1
==5623==    at 0x4C2D812: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x400942: sameWords (main.c:61)
==5623==    by 0x400A6F: main (main.c:85)
==5623==  Address 0x51fd045 is 0 bytes after a block of size 5 alloc'd
==5623==    at 0x4C2CD7B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x400913: sameWords (main.c:58)
==5623==    by 0x400A6F: main (main.c:85)
==5623== 
==5623== 
==5623== 4 errors in context 3 of 4:
==5623== Invalid read of size 1
==5623==    at 0x4C2D7B4: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x4009EC: sameWords (main.c:70)
==5623==    by 0x400A6F: main (main.c:85)
==5623==  Address 0x51fd093 is 0 bytes after a block of size 3 alloc'd
==5623==    at 0x4C2CD7B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x40092B: sameWords (main.c:59)
==5623==    by 0x400A6F: main (main.c:85)
==5623== 
==5623== 
==5623== 6 errors in context 4 of 4:
==5623== Invalid read of size 1
==5623==    at 0x4C2D7B4: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x40099E: sameWords (main.c:65)
==5623==    by 0x400A6F: main (main.c:85)
==5623==  Address 0x51fd045 is 0 bytes after a block of size 5 alloc'd
==5623==    at 0x4C2CD7B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5623==    by 0x400913: sameWords (main.c:58)
==5623==    by 0x400A6F: main (main.c:85)
==5623== 
--5623-- 
--5623-- used_suppression:      2 dl-hack3-cond-1
==5623== 
==5623== ERROR SUMMARY: 12 errors from 4 contexts (suppressed: 2 from 2)

The program should find out whether 2 strings are made from the same words (and ignore different sizes of letters). I am very sorry for the appearence of my code, I am quite new to programming and still trying to learn how to write it understandable, so I will briefly explain what is being done (at least I think so) where. The function WordInString takes one word after another from one string and then finds it in another. The word is being copied in dynamically allocated array, because I don't know how long the words could be. Then in function sameWords i copy the string in new arrays, so I could convert all the words to lower letters, and then I call function WordInString to search the strings for words. I main function I only call sameWords with 2 strings.

The code looks like this. Again sorry for the bad arrangement.

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


int WordInString(const char *a, const char *b)
{
    /* Selecting words one after another and finding the word in another strin.
    Dynamic allocation of word string, because words lengths are unknown.*/
    char * word=NULL;
    int previous = 0;
    int length=0;
    int wordlength=0;
    int i;

    if((strlen(a)==0) && (strlen(b)==0))
        {
        return 1;
        }

    i=0;
    while(1)
        {
        if (i>=wordlength) {wordlength+=250; word=(char*) malloc(wordlength*sizeof(char));}
        if(isalpha(a[i]))
            {
            if(!isalpha(previous))
                {
                length=0;
                }
            if(length<80) word[length++] = tolower(a[i]);
            }
            else
                {
                if(length>0)
                    {
                    word[length] = '\0';
                    if(strstr(b, word)==NULL)
                        {
                        return 0;
                        }
                    length=0;
                    }
                }
                if(a[i] == '\0') {break;}
                previous=tolower(a[i++]);
        }
    free(word);
    return 1;
}

int sameWords( const char * a, const char * b)
{
    int i=0, j=0;
    char * array1=(char*) malloc(strlen(a)*sizeof(char));
    char * array2=(char*) malloc(strlen(b)*sizeof(char));
    /* copy a and b strings to new string, that are not cons, so they can be changed      tolower */
    strcpy(array1, a);
    strcpy(array2, b);

    /* convert strings to lower letters  */
    for(i=0; i<strlen(array1); i++)
        {
        array1[i]=tolower(array1[i]);
        }
    for(j=0; j<strlen(array2); j++)
        {
        array2[j]=tolower(array2[j]);
        }
    /* calling WordInString to compare */
    if (WordInString(a, array2)==0) {return 0;}
    if (WordInString(b, array1)==0) {return 0;}
    free(array1);
    free(array2);
    return 1;
}


int main ( int argc, char * argv [] )
{
    int res;
    res=(sameWords("This is a string", "This string is a string"));
    return 0;
}

I would be very thankful for your help. I tried to look it up, but can't figure it out.

`malloc(strlen(a)*sizeof(char));` Size +1 need for the end of string the '\0' — BLUEPIXY, Dec 04 '13 at 13:30
[Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). Also, scaling by `sizeof (char)` is never good, it's just 1 so it adds nothing but confusion. — unwind, Dec 04 '13 at 13:33
So i should do malloc((strlen(a)+1)*sizeof(char)); ? And instead of sizeof, what would you suggest? The only thing i can think of is goind through both strings with some counter and then instead of sizeof put the counter there. — beranpa8, Dec 04 '13 at 13:37
Also if: `if (WordInString(a, array2)==0) {return 0;}` will not free array1 and array2. So you'll have a leak — Sanandrea, Dec 04 '13 at 13:43
But when I run it through Valgrind, it says that everything is freed. I free all the arrays in both functions. — beranpa8, Dec 04 '13 at 13:48
Consider also BLUEPIXY comment, he is right on the malloc size. — Sanandrea, Dec 04 '13 at 14:06
I replaced the strlen(a) with (strlen(a)+1), but now I have error while compilation: "comparsion between signed and unsigned integer expressions", on the line where is "for(i=0; i — beranpa8, Dec 04 '13 at 14:12
printf strlen(a) and strlen(b) at the beginning of function sameWords. Check this. Make i and j unsigned ints — Sanandrea, Dec 04 '13 at 14:13
When I print stlen(a) and strlen(b), it counts it goodly. For "This is a string" 16 and for "This string is a string" 23. — beranpa8, Dec 04 '13 at 14:17
`strlen()` returns `size_t` which is `unsigned`, so just defines your counters (`i`, `j`) also to be `size_t`. — alk, Dec 04 '13 at 14:19
And just remove any occurence of `sizeof(char) *` as it's defined to be `1`. — alk, Dec 04 '13 at 14:21
I redefined i and j to be unsigned and delted sizeof(char), but still am getting that segfault, I also replaced "char * array1=(char*) malloc(strlen(a));" with "char * array1=(char*) malloc((strlen(a)+1));" — beranpa8, Dec 04 '13 at 14:30
Thank you very much. The Valgrind now says no Errors and the segfault is gone. It was the problem with the strlen(a)+1 and signed i and j. Thank you for your time and help. — beranpa8, Dec 04 '13 at 14:35

score 1 · Accepted Answer · answered Jan 10 '17 at 14:20

1

malloc(strlen(a)*sizeof(char)); Size +1 need for the end of string the '\0' – BLUEPIXY

answered Jan 10 '17 at 14:20

Armali

18,255
14
57
171

C - Valgrind "Invalid read/write of size 1" errors while strcpy/strlen

1 Answers1