Where is mistake in this script ?

Question

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

  char *string = "<aa>RRRR|<aa>SSS|";
  char *cmp = "<aa>";
  char *p1;
  int nlen;

  main()
  {
    while ( p1 = strstr(string, "<aa>")  ) {
    nlen = 0;
    p1 = p1 + 4;
    while ( ( *p1 != '|' ) && ( *p1 != '\0') ) { p1 = p1 + 1; nlen++; };
    p1 = p1 - nlen;
    string = p1 + 1;
    char rr[200];
    strncpy(rr,p1,nlen);
    printf("%s\n", rr);
    }
  }

Wrong answer:

RRRR
SSSR

Answer 1

You're missing a null character to terminate the string. From Wikipedia's article on strncpy.

strncpy writes exactly the given number of bytes, either only copying the start of the string if it is too long, or adding zeros to the end of the copy to fill the buffer. It was introduced into the C library to deal with fixed-length name fields in structures such as directory entries. Despite its name it is not a bounded version of strcpy, it does not guarantee that the result is a null-terminated string. The name of the function is misleading because strncat and snprintf are respectively bounded versions of strcat and sprintf.

Adding the null character makes your program work:

strncpy(rr, p1, nlen);
*(rr+nlen) = '\0';   // this line
printf("%s\n", rr);

I think your code could have a risk of a buffer overflow because nlen could in theory be greater than 200. Though this is not an issue in the program in your question because the input string is so short.

You might also want to look at strncat which can be used to copy a string including the null character.

An alternative from the standard C library that will always append one null byte is to use strncat with an initially empty string as the destination.

Answer 2

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

main()
{
  char *string = "<aa>RRRR|<aa>SSS|"; /*make a copy below because string is const*/
  char *cmp = "<aa>";
  size_t cl = strlen(cmp); /* for shorter code below */

  char *s=strcpy(malloc(strlen(string)+1),string),*t, /* need test malloc-return */
       *last=strstr(s,cmp);
  if( !last ) return free(s),1;
  while( strstr(last+cl,cmp) )
    last=strstr(last+cl,cmp);
  for(t=strtok(s,"|");t;t=strtok(0,"|")); /* set all '|' to '\0' here */
  t=s;
  while( t<=last )
  {
    if( !strncmp(t,cmp,cl) )
      puts(t+cl);
    ++t;
  }
  return free(s),0;
}