0

Hello I am new to c and I am having some trouble getting my code to run, everything compiles fine but when I enter the first input the exe crashes. I am pretty sure the problem lies in either the memory allocation of my array or my use of pointers but I can not seem to figure out the issue.

Here are my 2 files.

main.c

    #include "stdheader.h"
    int main(int argc, char ** argv){
        char* s1;
        char* s2;
        char* s3;
        while (scanf("%c\n", *s1) != (EOF)){
            scanf("%c\n", *s2);
            LCS(*s1,*s2,*s3);
            printf("%c \n", *s3);
        }

    }

lcs.c

    #include "stdheader.h"
    void LCS(char*s1, char*s2, char*s3){
        int i, j, k;
        int L1= strlen(s1);
        int L2= strlen(s2);
        int **A = (int**)malloc(sizeof(int *)*(L1+1));
        for (i=0; i<= L1; i++){
            A[i]= malloc(sizeof(int)*(L2+1));
        }
        for (i=0; i<=L2; i++){
            A[L1][i]=0;
        }
        for (j=0; i<=L1; j++){
            A[j][L2] = 0;
        }
        for (i=L1-1; i >= -1; i--){
            for (j=L2-1; j>= -1; j--){
                if (s1[i]== s2[j]){
                    A[i][j]= 1 + A[i+1][j+1];
                }
                else
                    A[i][j] = fmax(A[i+1][j],A[i+1][j+1]);

            }
        }
        i=0; j=0; k=0;
        int true = 0;
        while(true=0){
            if (i>L1 || j>L2 || A[i][j]==0)
                true=1;
            if (s1[i] == s2[j]){
                s3[k]=s1[i];
                i++; j++; k++;
            }
            else if (A[i][j+1] >= A[i+1][j])
                j++;
            else
                i++;
        }
        for(i=0; i<= L1; i++){
            free(A[i]);

        }
        free(A);
        s3[k]= '\0';

    }
Cwsager
  • 1
  • 1
  • Passing data having wrong type i.e. Passing data having `char` to `%c`, which calls for `char*`, to `scanf()` invokes undefined behavior*. You also invokes another *undefined behavior* for using (dereferencing) value of uninitialized variable with automatic storage duration. – MikeCAT Feb 29 '16 at 03:29
  • You are also passing data having wrong type to `LCS()`. Turn on compiler warnings and listen to it. – MikeCAT Feb 29 '16 at 03:30
  • Also note that they say [you shouldn't case the result of `malloc()` in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – MikeCAT Feb 29 '16 at 03:31
  • 1
    `s1`, `s2`, and `s3` are *unallocated* pointers. They have no memory to store anything in. You either need to dynamically allocate with e.g. `malloc` or declare them as character arrays of sufficient size to hold what you are attempting to put in them (e.g. `s1[64]`)(`+1` for the *nul-terminating* character). – David C. Rankin Feb 29 '16 at 03:31
  • 1
    @MikeCAT typo `cast the result` instead of `case...` – David C. Rankin Feb 29 '16 at 03:33
  • @DavidC.Rankin Did you mean uninitialized? The pointers are definitely allocated. One must be careful with terminology around pointers to avoid misunderstandings. – user253751 Feb 29 '16 at 04:28
  • Right you are. *uninitialized* was the intended term. (I must have been hungry at the time `:)` – David C. Rankin Feb 29 '16 at 04:34
  • what is `stdheader.h`? – user3629249 Feb 29 '16 at 05:57
  • when compiling, always enable all the warnings, then fix those warnings. (for `gcc`, at a minimum use: `-Wall -Wextra -pedantic` I also use: `-Wconversion -std=gnu99` ) for a start, the parameters `argc` and `argv[]` are not used, so should use a main() signature of: `int main( void )` – user3629249 Feb 29 '16 at 06:01
  • regarding this line: `while (scanf("%c\n", *s1) != (EOF))`, 1 the check for the returned value from `scanf()` should be for the number of `format conversion sequences, not a specific problem like EOF. 2) the `s1` is already a pointer (which, BTW: currently points to trash ) so needs something specific to point toward. Otherwise it is undefiend behaviour and can (as you saw) lead to a seg fault event.) the line should be written like: `while ( 1 == scanf( "%c\n", s1 ) )` – user3629249 Feb 29 '16 at 06:09
  • when calling a memory allocation function (malloc, calloc, realloc), always check (!=NULL) the returned value to assure the operation was successful. In C, the returned value has type `void*` so can be assigned to any pointer. Casting the returned value just clutters the code, making understanding, debugging and maintenance more difficult. Note: `sizeof()` returns a `size_t` and `L1` is an int, what can create problems suggest declaring `L1` as `size_t L1;' – user3629249 Feb 29 '16 at 06:11
  • for ease of readability and understanding by us humans, 1) please follow the axiom: *only one statement per line and (at most) one variable declaration per statement.* 2) please use meaningful variable names. The variable name should indicate contents or usage (or better) both. – user3629249 Feb 29 '16 at 06:15
  • these kinds of lines: `scanf( "%c\n", s2 );` are inputting a single character, not a NUL terminated array of char. So the lines in LCD(), that call `strlen()`, like `int L1= strlen(s1);` will fail. (amongst other things, there was only a single char input not an array of characters. Perhaps you meant the call to `scanf()` to be similar to: `while (scanf( "%s\n", s1) == 1 )`, – user3629249 Feb 29 '16 at 06:26
  • cont: which still has a problem because the use could overflow any input buffer that is supplied (currently `s1` should be pointing to a memory buffer). If `s1` pointed to a memory buffer of 25 char, then the line should look like: `while (scanf( "%24s\n", s1) == 1 )` to allow for `scanf()` automatically appending a NUL byte to the input. – user3629249 Feb 29 '16 at 06:27
  • the variable `true` should be eliminated. the insert `#include ` And never assign a value to `true` – user3629249 Feb 29 '16 at 06:30
  • exit a `while(true)` loop using the `break;` statement, So far, I have only scratched the surface of the problems with the posted code. Strongly suggest: when compiling, enable all the warnings, then fix those warnings. – user3629249 Feb 29 '16 at 06:32

0 Answers0