Issue finding number of occurrences of a given substring in a string

Question

The below code always returns number of matching sub strings as zero.There are no errors in the code and i am not sure where have i gone wrong logically.

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

int main()
{ 
    int i,j,len ,k ,count, num ;
    char str[100],sub[100],comp[100] ; 
    // sub is the sub string .
    printf("Please enter the string") ; 
    gets(str) ;
    printf("Enter the substring to be searched for") ;
    gets(sub) ;
    len=strlen(sub) ;
    for ( i=0 ; i < strlen(str) - len ; i++ ) 
    //Goes till length of string - length of sub string so that all characters can be compared.
      {  
         num = i + len ;
         for ( j=i,k=0 ; j<num ; j++, k++ )
         //Loop to store each sub string in an array comp.
           {
             comp[k]=str[j] ;
           }
         if ( strcmp(comp,sub) == 0 )
            { count++ ; }
     }
    printf("no of occurances is:%d",count) ;
    return 0 ;
}

This code doesn't make any sense. Why do you need to make a copy of the sub string in the first place? — Lundin
– Lundin, Commented Jan 27, 2016 at 14:09
use fgets instead of gets unless there is a very good reason. — ForeverStudent
– ForeverStudent, Commented Jan 27, 2016 at 14:15
@ForeverStudent: Which reason would rectify to use an unsafe function with not more features? — too honest for this site
– too honest for this site, Commented Jan 27, 2016 at 14:31
@Olaf: stupid ones we all heard before: fgets not allowed by assignment, fgets not supported by our custom compiler/runtime, etc — ForeverStudent
– ForeverStudent, Commented Jan 27, 2016 at 15:15

dbush · Accepted Answer · 2016-01-27 14:15:20Z

2

As mentioned in the comments, when constructing comp, you're not adding a terminating null byte at the end. Because the rest of comp is not initialized, you invoke undefined behavior when calling strcmp.

Add the null byte at the end of the inner for loop will fix the problem:

     for ( j=i,k=0 ; j<num ; j++, k++ )
     //Loop to store each sub string in an array comp.
       {
         comp[k]=str[j] ;
       }
     comp[k] = '\0';

Actually, rather than creating a separate substring, just use strncmp, which compares up to a certain number of characters:

for ( i=0 ; i < strlen(str) - len ; i++ ) 
//Goes till length of string - length of sub string so that all characters can be compared.
  {  
     if ( strncmp(&str[i],sub,strlen(sub)) == 0 )
        { count++ ; }
 }

Also, don't use gets, as that is prone to buffer overflows. Use fgets instead.

edited Jan 27, 2016 at 14:15

answered Jan 27, 2016 at 14:08

dbush

233k27 gold badges261 silver badges334 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

chux Over a year ago

Note: As the compare will not exceed the length of the strings, code code use memcmp(&str[i], sub, ...) Likely faster with long strings YMMV.

Rahul Mayuranath Over a year ago

That was very helpful for a beginner like me man. Thank you !

dbush Over a year ago

@RahulMayuranath Glad I could help. Feel free to accept this answer if you found it useful.

Rahul Mayuranath Over a year ago

After making the above changes( using fgets instead of gets and adding a terimal null byte) I am now getting an output digit in excess of 30000 for every test case. What is happening ?

dbush Over a year ago

@RahulMayuranath We would need to see the code. Please post that as a separate question.

|

Sagar Patel · Accepted Answer · 2016-01-28 13:19:26Z

0

Try changing your for loop from this :

for ( i=0 ; i < strlen(str) - len ; i++ )

to

for ( i=0 ; i <= strlen(str) - len ; i++ )

answered Jan 28, 2016 at 13:19

Sagar Patel

8521 gold badge11 silver badges22 bronze badges

Collectives™ on Stack Overflow

Issue finding number of occurrences of a given substring in a string

2 Answers 2

6 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related