c strcat with pointers

I want to code this example function in C (please don't judge the purpose of that piece of code, it's only for learning the C). It should use dynamic allocation of memory to not consume more memory, than is neccesary.

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

char ** LCS ( char * s1, char * s2 ) {
    int i, ii, myI;
    char ** commonSequence = (char**) malloc(1 * sizeof (char**));
    *commonSequence = NULL;

    int commonChars = 0;
    for (i = 0; i < (int)strlen(s1);i++) {
        for (ii = 0; ii < (int)strlen(s2); ii++) {
            if (s1[i] == s2[ii]) {
                commonChars++;
                if (commonChars > 1) {
                    commonSequence = (char**) realloc(commonSequence, (commonC开发者_如何学运维hars+1) * sizeof (char**));
                    strcat((char*)commonSequence, (const char *)s1[i]);
                }
                i++;
            }
        }
    }

    return commonSequence;
}

int main ( int argc, char * argv [] ) {
    char ** output = LCS((char*)"AATK", (char*)"AABC");

    printf("\n\nCommon: %s", *output);
    free(output);

    return 0;
}

The code above is causing the "Segmentation fault" error (perhaps something wrong with memory allocation or sth like that). When I run it in Valgrind, it outputs this:

==29381== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 3 from 3)
==29381==
==29381== 1 errors in context 1 of 1:
==29381== Invalid read of size 1
==29381==    at 0x4028A80: strcat (in /usr/lib/valgrind/vgpreload_memcheck-x86-l                                                                                          inux.so)
==29381==    by 0x80485BA: LCS(char*, char*) (main.cpp:19)
==29381==    by 0x8048626: main (main.cpp:31)
==29381==  Address 0x41 is not stack'd, malloc'd or (recently) free'd

Probably I'm not using the strcat function properly... But I actually don't know what to fix...

---

You're casting s1[i], which is a char, into a meaningless const char * pointer:

strcat((char*)commonSequence, (const char *)s1[i]);

You're also converting commonSequence from a char ** to a char *, which won't work. You probably want something like:

strcat(*commonSequence, ((const char *)s1)[i]);

But there's really something wrong in the way you allocate memory for commonSequence. It looks like you're building a list of strings without allocating the strings themselves.

---

Your use of malloc is bogus, but since the numbers happen to work out right, the error likely won't be caught. This line:

char ** commonSequence = (char**) malloc(1 * sizeof (char**));

should read:

char ** commonSequence = malloc(1 * sizeof (char*));

or preferably:

char ** commonSequence = malloc(1 * sizeof *commonSequence);

The final form is the nicest because it does not require any changes if you change the type of commonSequence. As for the actual problem with the original line, you need to allocate based on the size of the object you will be pointing to, not the size of the pointer that points to it. In practice, since both are pointers they'll be the same size on modern real-world machines, but it's still a serious conceptual error that could bite you when you apply the incorrect pattern to non-pointer types. For instance this would result in memory corruption/undefined behavior:

struct foobar *foo = malloc(1 * sizeof (struct foobar *));

because it would only allocate 4 or 8 bytes instead of the size of the structure.

---

 strcat((char*)commonSequence, (const char *)s1[i]);

strcat needs a pointer, what you are doing there with (const char *)s1[i] is that you are accessing the character and then giving it in as a pointer. That is to say, instead of the address where the strcat should start, you are giving it the value of the character.

---

You're allocating memory for a pointer-to-a-pointer to a string, but not for the string itself. The first argument to strcat() seems like it should be *commonSequence.

---

You need to pass tha address of s1[i]. Simply casting it will not make it a valid address. Equally casting a char** to a char* will not yield the result you expect. You should dereference it to access the char* pointer stored in the char**

strcat( *commonSequence, &s1[i] );

That said, the rest of the code looks suspect. What is it you are actually trying to acheve I wonder? But you explicitly asked us not to question that!