writing a substring function in c

Question

char *substring(char *string, int index, int length)
{
    //int counter = length - index;
    int counter = 0;
    for(;string[index] != string[length];index++, counter++);
    printf("\n%d\n", counter);
    char *array = malloc(sizeof(char) * counter);
    if(array != NULL)
    {
        ///while(string[index] != string[length])
        while(index != length)
            {
                array[index] = string[index];
                index++;
                array++;
            }
    }
    else
        puts("Dynamic allocations failed\n");
    return array;
}   

1 - I've commented out initializing counter with "length - index" because I didn't feel comfortable with it(I also kinda like one line for loops:) ). So, can I count on counter if I used it in this simpler way.

2 - My problem with this code is that It doesn't return anything. I try to printf the result but nothing is printed and when I assign the result of the function to a char *, I get an error saying that I cannot assign a void to char *. But how is it returning void?

3 - Can I write this function with pointer arithmetic and without any array indexing at all?!

4 - Can I mutate the char *array ?!. I'm asking this because I thought char * cannot be mutated, but I've read a code that correctly mutated a char *. Or is it that I'm confusing a regular char * and a string?

Note: I do not want to use string library functions

Answer 1

There are several problems with your code.

for(;string[index] != string[length];index++, counter++);

First, in the above line, you need to subtract one from length since arrays are 0-indexed.

This line will also break if the last character of the substring is repeated somewhere else in the substring. For example aba where index = 0, length = 3. Your loop will immediately stop since string[0] == string[2] even though you did not reach the end of the substring.

However, the entire loop for counting substring length is unnecessary, just use length!

char* array = malloc(length + 1);

Note that you need +1 to include the null terminator that is standard in C-strings.

Next, this line won't work if index is not 0.

array[index] = string[index];

index is the index into string, not the index into array. You should use a different variable for your iterator that starts at index and then increments. Then you can subtract index from the iterator to get the actual index in the substring array.

int i = index;
while(i < length)
{
    array[i - index] = string[i];
}

Also note that this line in the loop is unnecessary and breaks your code.

    array++;

Your iterator is being incremented so you have no need to increment the array pointer. Also, if you increment array directly, then when you return it, where is it pointing? It's pointing to the end of the string, not the beginning, so of course you will get no output.

Finally, don't forget to add the null terminator. After your loop, since your iterator will now conveniently point to the last index, just do

array[i] = '\0';

As extra, since you are specifically creating a new string to hold the substring and not modifying the original string pointer, you should declare the argument for string as const. Ex: const char *string. This is not required however there are a number of reasons that const-correctness is important.

If you make the above changes your code should work just fine. I am not posting completed code as I think making the changes yourself is a valuable exercise.

Answer 2

1 - You can use the for loop, but it is a slow way. Simple subtraction does the same job instantly, I see no reason not to feel comfortable with it.

2 - Printf - remove the semicolon at the end of the for loop, about the pointer:

void * is an universal pointer type. You should cast it to char *, like so:

char *array = (char *)malloc(sizeof(char) * counter);

3 - Yes, *(ptr + 3) is equivalent to ptr[3]. In most implementations it is a bit faster to use pointers as well.

4 - Yes, you can modify the memory pointed by a char * pointer. It would modify the original string though, I'm not sure if that's your goal.

Answer 3

Worked 100%

    #include "stdafx.h"
    #include "stdlib.h"
    void substring(char * src, char * dst, int index, int length);
    int _tmain(int argc, _TCHAR* argv[])
    {
        char orig[]="test";
        char * newchar = (char*)malloc(2);//1 char + null

        substring(orig, newchar, 1, 1);

        printf(newchar);

        free(newchar);
        system("pause");
        return 0;
    }

void substring(char * src, char * dst, int index, int length)
{
    // Assign src_index to our initial index
    // Assign a new counter for dst position
    // We want only up to the length hence index+length
    // Increment src_index and dst_index
    for(int src_index = index, dst_index = 0; src_index < index + length; src_index ++, dst_index++)
    {
        dst[dst_index] = src[src_index];
    }
    dst[length++] = '\0'; // Null terminate the string, we already accounted for this above.
}