tags:

views:

49

answers:

4

Hello all,

I have this C function which attempts to tell me if a sub string is contained in a string.

int sub_string(char parent [1000], char child [1000]){

  int i;
  i = 0;
  int parent_size = (int) strlen(parent);
  int child_size = (int) strlen(child);
  char tempvar [child_size];

  int res;
  res  = 1;

  while(i<(parent_size - child_size + 1) && res != 0){

   strncpy(tempvar, parent + i, child_size);

   if(strcmp(tempvar, child)==0){
      res = 0;
   }
   i++;

   memset(tempvar, 0, child_size);
  }

  memset(tempvar, 0, sizeof(tempvar));
  return res;
}

Now the strange thing is, when I pass a string "HOME_DIR=/tmp/" and "HOME_DIR" it returns a 0 the first time round, but after I call this function again, it returns a 1 to say it hasn't found it!!

I am guessing this is a memory issue, but I can't tell where, I would appreciate any help on this.

+1  A: 

Is there any reason you can't use the strstr function? Otherwise there are some things you should clean up in your code. For starters since you are limiting the length of the arrays coming in to 1000 characters you should use strnlen instead of strlen with a limit of 1000. You should also create you should zero out the tempvar array before you start copying into it. If parent is not null terminated you could run off the end of the array in your while loop. I would also suggest using strncmp and giving a length limit (in general if you are using the C string library you should use the 'n' version of the functions i.e. strnlen instead of strlen so that you put a bounding length on the operation, this helps to protect buffer overflows and potential security holes in your code).

pstrjds
Cheers pstrjds for strstr!
Kay
np - I always try to use a standard library function when one is available.
pstrjds
+1  A: 

I have noticed some issues with this program:

  1. Use pointers instead of fixed char arrays. This is more space optimal. So your function definition becomes int sub_string(char *parent, int parent_len, char *child, int child_len). Please note that since I pass pointers I also need to pass the length of the string so I know how much to traverse. So now you access your string like so *(parent+i) in a loop.
  2. i<(parent_size - child_size + 1) This condition looks a bit dicey to me. Let's say parent is 100 in len & child is 75. so this expression becomes i<26. Now your loop will terminate when i>26. So tempvar would have the parent_string till index 25. So how does this work again?
MovieYoda
+1  A: 

One problem is:

char tempvar [child_size];

strcmp below will compare child_size+1 characters (incl. terminating '\0'), therefore its undefined behaviour. Do you know the C-standard functions strstr and strncmp?

I failed to find a substring function to include - many thanks for strstr!
Kay
A: 

sizeof(tempvar) does not return child_size.

Tim