C Substring Function <Resolved />

[señorbadger]

Hello i have tried to write a substring function in C

but it wont compile

i still learning C but can someone help me get this working please

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

typedef char* string;


string substring(string haystack,int start,int length)
{
char* substr;
int c, i;
  if (start > strlen(haystack) || length > strlen(haystack) || (start + length) > strlen(haystack))
    {
      return(0);
    }
    else
    {
    i = 0;
      for(c=start;c<(start + length);c++)
        {
          substr[i] = haystack[c];
          i++;
        }
      return(substr);
    }
} 

[CornedBee]

You're dealing with memory problems that about every single C newbie has.

First, a thing of convention. Among the C-like languages, it seems that a general trend goes toward two names for substring functions: substr and substring. Java's String class uses substring, C++'s std::string uses substr, PHP uses substr, JavaScript has both. The interesting thing about them is that generally, substr takes a start index and a length parameter (like yours) and substring takes a start index and an end index parameter.
So I suggest you call your function substr.


But that's not the point.

Here's another thing that's not the point: calling strlen three times is very inefficient. Call it once and store the value.

I see no reason why your function should fail to compile. Can you give us your error message?
But I see a big reason why your function should fail to run correctly.

As I said, your problem is memory allocation.
char *substr;
is merely a pointer, and it points to nothing. You then try to fill the area it points to with something. But it doesn't point to anything. The results are, as the C community calls it, undefined. Anything at all, from working over crashing over messing up totally different parts of your program to destroying the computer. Admittedly, the last one is rather unlikely, but the three others are likely, and which one actually occurs might change from invocation to invocation!

What you need is memory. There is one invalid way that is often used by newbies, and that's to make substr a local char array. This local array is local and temporary, though, so you must not return a pointer to it.

There are thus two valid ways.
One is to allocate memory using malloc inside the function:

  else
  {
    substr = malloc(length+1);

Personally, I can't stand functions that allocate memory and don't have "alloc" in their name. This requires the caller to know that they receive a pointer to allocated memory, and it requires them to free() it! It's too easy to forget about that, and then you have a memory leak.

The second way is the typical C way: require the caller to pass a buffer:

char *substr(char *buf, const char *source /* haystack is for find operations */, int start, int length)
{
  /* ... */
  for(...) {
    buf[i] = source[c];
  }
}

Fine. The only issue is, once you're there, you don't need your function anymore, because there's already a function in the C library that does it: strncpy.

strncpy(buffer, souce+start, length);

What strncpy doesn't do is bounds checking, so you might want to do this:

char *substr(char *buf, const char *source, size_t start, size_t length)
{
  size_t slen = strlen(source);
  if(start+length > slen) {
    buf[0] = '\0';
    return buf;
  } else {
    return strncpy(buf, source+start, length);
  }
}

This is about the best checking substr implementation you'll get.

Note these things:
[list=1][*] size_t should be the type you always use for sizes, such as offsets into an array, lengths in an array etc. It is an unsigned integral type, which means that it can't be negative.[*] Use const for strings that will not get modified, such as the source string here.[*] If x > c or y > c and x,y >= 0 then x+y > c. Thus, it suffices to check only start+length against slen.[*] C functions that take a buffer also return it, so that they can be chained together. I do this by returning buf in one if-branch and the return value of strncpy (which follows the same convention) in the other.[*] I don't return a NULL-pointer if the substring is empty (out of bounds). NULL pointers might introduce problems. Instead, I return a zero-length string by setting the very first character in the buffer to the string terminator.[/list=1]

[señorbadger]

Thanks Mate, i really don't like C yet but have to learn it for uni

thanks again