C: substring function with return

Question

I'm learning C programming language and I'm trying to create a function which makes a substring and then returns it. I found this code here:

void substring(char s[], char sub[], int p, int l) {
   int c = 0;

   while (c < l) {
      sub[c] = s[p+c-1];
      c++;
   }
   sub[c] = '\0';
}

I want to get rid of the char sub[] in parameters and instead return a string created inside of the function. This is the closest (I think) think I got, but I always get a segmentation fault (core dumped) runtime error.

char substring(char s[], int p, int l) {
   int c = 0;
   char sub[1000];

   while (c < l) {
      sub[c] = s[p+c-1];
      c++;
   }
   sub[c] = '\0';

   return *sub;
}

Thanks for any help

You cannot return a locally declared string – it disappears as soon as the function ends. That's why the original code uses a second pointer. If you want to return persistent memory, use malloc. — Jongware
– Jongware, Commented Sep 29, 2015 at 11:20
For performance and memory workload reason, I would just use the original string as long as possible. In your case I would just return the pointer of this sub string which simplifies everything to char* SubString = s + p; where p is the start position. IF you really need a copy later time, I wouldn't reinvent the wheel and use one of predefined methods like memcpy(MySubStringCopy, SubString, l); Variable MySubStringCopy could be locally stack allocated or heap allocated. — bkausbk
– bkausbk, Commented Sep 29, 2015 at 12:02

Ivaylo Strandjev · Accepted Answer · 2015-09-29 11:28:55Z

2

If you want to return a string you should not return a single char, but a char* instead. And as sub is local character array you can't simply return sub. You'll need to allocate dynamic memory if you insist on returning a pointer from your function. It will then be the caller's responsibility to free the memory. Thus you should do something like this:

// NOTE: caller takes ownership of the returned value
char* substring(char s[], int p, int l) {
   int c = 0;
   char* sub = malloc(l + 1);

   while (c < l) {
      sub[c] = s[p+c-1];
      c++;
   }
   sub[c] = '\0';

   return sub;
}

NOTE: thanks to @MOehm's comment I have noticed that there is no need to allocate a string of fixed size 1000 when the size is known aforehand. Still this may result in a logical change and thus you may need to alter the rest of your code.

edited Sep 29, 2015 at 11:28

answered Sep 29, 2015 at 11:22

Ivaylo Strandjev

71.4k18 gold badges130 silver badges183 bronze badges

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

7 Comments

M Oehm Over a year ago

Yes, but why allocate a fixed size when the length is known?

Ivaylo Strandjev Over a year ago

@MOehm I did not notice that I simply re-wrote the code as it was. I will reflect your comment. Still changing the allocation size may reflect on the other code(if it expects size of 1000).

M Oehm Over a year ago

Thanks. I think the alloc to size is better. If the code really is "get char buffer of size 1000 and fill the first bytes with the substring" then the original variant that passes the buffer should be preferred. I guess the 1000 was just an arbitrary value to mean "big enough".

Ivaylo Strandjev Over a year ago

@MOehm so do I. Still I have added a NOTE to point to this slight change just in case

sveatlo Over a year ago

Is it possible to rewrite your code, so it doesn't use malloc?

|

Tamás Zahola · Accepted Answer · 2015-09-29 11:24:25Z

Problems:

Your return type is char, which means you're returning a single character. This should be char* instead, because you want to return a pointer to the beginning of the string.
You're using a locally declared array to hold your result. Local variables reside on the stack and will be unavailable once the function returns. You need to allocate heap memory with malloc, store the substring in that memory block, and return the pointer. Note that this means that the caller would need to take ownership of this block (i.e. freeing it after it isn't needed).

Notice that this implementation unnecessarily forces the caller to use heap memory. If the caller wants to put the string somewhere else (e.g. in a memory mapped file), he would need to copy it. The original implementation used an inout parameter probably to avoid these kind of compromises.

Lundin · Accepted Answer · 2015-09-29 12:02:39Z

1

As already mentioned in other answers, you can't return pointers to local variables. The best way is in fact not to concern yourself with memory allocation at all, but instead leave that to the caller:

void substring (size_t index, 
                size_t length, 
                const char source[], 
                char dest[length+1])
{
  memcpy(dest, source + index, length);
  dest[length] = '\0';
}

answered Sep 29, 2015 at 12:02

Lundin

220k47 gold badges281 silver badges446 bronze badges

Collectives™ on Stack Overflow

C: substring function with return

3 Answers 3

7 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

7 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related