1

I am dynamically allocating the 2D array like this:

char ** inputs;
inputs = (char **) malloc(4 * sizeof(char));

After doing this I started having the problem with the string. I printed the string before and after allocating the 2D-array:

printf("%s\n", str);
char ** inputs;
inputs = (char **) malloc(4 * sizeof(char));
printf("%s\n", str);

But I get strange output:

before: input aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa with len  34
after: input aaaaaaaaaaaaaaaaaaaaaaaaaaaa with len  29

Why the length is changed? I've searched through stackoverflow and other websites but couldn't find reasonable answer for that.

Here is my all function call:

int main(int argc, char const *argv[])
{
    /* code */

    mainProcess();
    printf("\nEnd of the program\n");
    return 0;
}

//  Reading the input from the user
char * getInput(){
    printf("Inside of the getInput\n");
    char * result;
    char * st;
    char c;
    result = malloc(4 * sizeof(char));
    st = malloc(4 * sizeof(char));
    // code goes here 
    printf("$ ");
    while(3){
        c = fgetc(stdin);
        if(c == 10){
            break;
        }
        printf("%c", c);
        result[length] = c;
        length++;
    }   

    result[length] = '\0'; 

    return result;
}

void mainProcess(){
    char * input;

    printf("Inside of Main process\n");

    input = getInput();
    printf("\nthis is input %s with len  %d\n", input, strlen(input));

    splitInput(input);

    printf("\nthis is input %s with len  %d\n", input, strlen(input));
}



char ** splitInput(const char * str){
    char ** inputs;
    inputs = NULL;
    printf("inside split\n");
    printf("%s\n", str);

    inputs = (char **) malloc( sizeof(char));


    // free(inputs);
    printf("------\n"); // for testing
    printf("%s\n", str);
    if(!inputs){
        printf("Error in initializing the 2D array!\n");
        exit(EXIT_FAILURE);
    }

    return NULL;
}
Zafar
  • 1,111
  • 2
  • 11
  • 23
  • My guess is that `str` is not initialized to point to anything, so anything could happen. Can't tell if you don't post all the code. – imreal Apr 18 '17 at 15:47
  • It is initialized before the function call. – Zafar Apr 18 '17 at 15:49
  • 4
    You haven't allocated a 2-dimensional array. – Barmar Apr 18 '17 at 15:50
  • `char **` should be a pointer to an array of pointers. You should use `malloc(N * sizeof(char *))`, and then you need to allocate a pointer for each row. – Barmar Apr 18 '17 at 15:51
  • You've allocated enough space in `inputs` for half of a 64-bit pointer. That's not what you intended, I suspect. You might not really be wanting `char **inputs;` — it looks more like you want `char *inputs;`, but even then, 4 bytes isn't room for very much of a string (3 characters and a null byte). Revisit your memory allocation — it is not yet correct. – Jonathan Leffler Apr 18 '17 at 15:52
  • If I had to guess, you've allocated too little space for `str` as well – cbr Apr 18 '17 at 15:53
  • Please post an MCVE that demonstrates the problem. You almost certainly have other problems outside the snippet you posted. – Barmar Apr 18 '17 at 15:54
  • I don't allocate 2d array (using for loop) because I don't know the size of the input. – Zafar Apr 18 '17 at 16:14
  • What is `while(3){` supposed to achieve? – Yunnosch Apr 18 '17 at 16:50
  • `while(non-zero){do smth}` I picked random non-zero number to loop while it is true. – Zafar Apr 18 '17 at 16:52
  • `inputs = (char **) malloc(4 * sizeof(char));` allocate **4-bytes** of storage for `inputs` (a *pointer-to-pointer-to-char*) On x86_64 systems, pointer addresses are generally `8-bytes`, if that is your case, you haven't allocated enough storage to hold the address of a single pointer to char. On x86, with `4-byte` pointers, you would have allocated enough storage for `1` pointer. Generally with *pointer-to-pointer-to char*, you allocate pointers (e.g. `char **inputs = malloc (nptrs * sizeof *inputs);` and then allocate storage for each string, `inputs[x] = malloc (strlen (str) + 1);` – David C. Rankin Apr 18 '17 at 16:56
  • what criteria are you trying to split a string? The function splitInput() returns double pointer but I don't see a line where it returns a double pointer. – Nguai al Apr 18 '17 at 16:58
  • 1
    Also, there is NO need to cast the return of `malloc`, it is totally unnecessary. `malloc` returns a **memory address** (e.g. `void*`) which can be freely assigned to any pointer. See: [**Do I cast the result of malloc?**](http://stackoverflow.com/q/605845/995714) for thorough explanation. – David C. Rankin Apr 18 '17 at 16:59
  • Did you declare (i.e. provide prototypes of) the functions which you call before defining them? The compiler has to do a lot of type guessing that way. – Yunnosch Apr 18 '17 at 17:00
  • @Nguaial first I have to figure out why the string is truncated then I will return double array with split string – Zafar Apr 18 '17 at 17:01
  • @Yunnosch Yes, I did declare them before the main. The issue is in allocating the 2D array – Zafar Apr 18 '17 at 17:02
  • 1
    YOU ARE ALLOCATING A BLOCK OF MEMORY FOR A **pointer-to-pointer-to-char** NOT A 2D ARRAY. Let that sink in. They are two completely different things. You can simulate a 2D array by properly indexing a **pointer-to-pointer-to-char** but that does NOT make it a 2D array. – David C. Rankin Apr 18 '17 at 17:04
  • Please update your MCVE after achieving warning-free compilation, using strict compilation, e.g. `gcc -Wall`. – Yunnosch Apr 18 '17 at 17:07
  • @Yunnosch I did, no warnings – Zafar Apr 18 '17 at 17:08
  • @DavidC.Rankin what is your proposal. I don't know into how many words will I split the string; therefore, I can't declare exact size. – Zafar Apr 18 '17 at 17:10
  • Please update **the MCVE you provided here**, to a version which will compile warning-free, using strict compilation, e.g. `gcc -Wall`. – Yunnosch Apr 18 '17 at 17:13

2 Answers2

3

It is not entirely clear what you are trying to accomplish, but it appears you are attempting to read a line of text with getInput and then you intend to separate the input into individual words in splitInput, but are not clear on how to go about doing it. The process of separating a line of text into words is called tokenizing a string. The standard library provide strtok (aptly named) and strsep (primarily useful if you have the possibility of an empty delimited field).

I have explained the difference between a 2D array and your use of a pointer-to-pointer-to-char in the comments above.

To begin, look at getInput. One issue that will give you no end of grief is c must be type int or you cannot detect EOF. In addition, you can simply pass a pointer (type size_t) as a parameter and keep count of the characters in result and avoid the need for strlen to get the length of the returned string. You MUST use a counter anyway to insure you do not write beyond the end of result to begin with, so you may as well make the count available back in the calling function e.g.

char *getInput (size_t *n)
{
    printf ("Inside of the getInput\n");
    char *result = NULL;
    int c = 0;      /* c must be type 'int' or you cannot detect EOF */

    /* validate ALL allocations */
    if ((result = malloc (MAXC * sizeof *result)) == NULL) {
        fprintf (stderr, "error: virtual memory exhausted.\n");
        return result;
    }

    printf ("$ ");
    fflush (stdout);    /* output is buffered, flush buffer to show prompt */

    while (*n + 1 < MAXC && (c = fgetc (stdin)) != '\n' && c != EOF) {
        printf ("%c", c);
        result[(*n)++] = c;
    }
    putchar ('\n');     /* tidy up with newline */

    result[*n] = 0; 

    return result;
}

Next, as indicated above, it appears you want to take the line of text in result and use splitInput to fill a pointer-to-pointer-to-char with the individual words (which you are confusing to be a 2D array). To do that, you must keep in mind that strtok will modify the string it operates on so you must make a copy of str which you pass as const char * to avoid attempting to modify a constant string (and the segfault).

You are confused in how to allocate the pointer-to-pointer-to-char object. First you must allocate space for a sufficient number of pointers, e.g. (with #define MAXW 32) you would need something like:

    /* allocate MAXW pointers */
    if ((inputs = malloc (MAXW * sizeof *inputs)) == NULL) {
        fprintf (stderr, "error: memory exhausted - inputs.\n");
        return inputs;
    }

Then as you tokenize the input string, you must allocate for each individual word (each themselves an individual string), e.g.

        if ((inputs[*n] = malloc ((len + 1) * sizeof *inputs[*n])) == NULL) {
            fprintf (stderr, "error: memory exhausted - word %zu.\n", *n);
            break;
        }
        strcpy (inputs[*n], p);
        (*n)++;

note: 'n' is a pointer to size_t to make the word count available back in the caller.

To tokenize the input string you can wrap the allocation above in:

    for (char *p = strtok (cpy, delim); p; p = strtok (NULL, delim))
    {
        size_t len = strlen (p);
        ...
        if (*n == MAXW)     /* check if limit reached */
            break;
    }

Throughout your code you should also validate all memory allocations and provide effective returns for each function that allocates to allow the caller to validate whether the called function succeeded or failed.

Putting all the pieces together, you could do something like the following:

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

#define MAXC 256    /* constant for maximum characters of user input */
#define MAXW 32     /* constant for maximum words in line */

void mainProcess();

int main (void)
{
    mainProcess();
    printf ("End of the program\n");

    return 0;
}

char *getInput (size_t *n)
{
    printf ("Inside of the getInput\n");
    char *result = NULL;
    int c = 0;      /* c must be type 'int' or you cannot detect EOF */

    /* validate ALL allocations */
    if ((result = malloc (MAXC * sizeof *result)) == NULL) {
        fprintf (stderr, "error: virtual memory exhausted.\n");
        return result;
    }

    printf ("$ ");
    fflush (stdout);    /* output is buffered, flush buffer to show prompt */

    while (*n + 1 < MAXC && (c = fgetc (stdin)) != '\n' && c != EOF) {
        printf ("%c", c);
        result[(*n)++] = c;
    }
    putchar ('\n');     /* tidy up with newline */

    result[*n] = 0; 

    return result;
}

/* split str into tokens, return pointer to array of char *
 * update pointer 'n' to contain number of words
 */
char **splitInput (const char *str, size_t *n)
{
    char **inputs = NULL,
        *delim = " \t\n",   /* split on 'space', 'tab' or 'newline' */
        *cpy = strdup (str);

    printf ("inside split\n");
    printf ("%s\n", str);

    /* allocate MAXW pointers */
    if ((inputs = malloc (MAXW * sizeof *inputs)) == NULL) {
        fprintf (stderr, "error: memory exhausted - inputs.\n");
        return inputs;
    }

    /* split cpy into tokens (words) max of MAXW words allowed */
    for (char *p = strtok (cpy, delim); p; p = strtok (NULL, delim))
    {
        size_t len = strlen (p);

        if ((inputs[*n] = malloc ((len + 1) * sizeof *inputs[*n])) == NULL) {
            fprintf (stderr, "error: memory exhausted - word %zu.\n", *n);
            break;
        }
        strcpy (inputs[*n], p);
        (*n)++;

        if (*n == MAXW)     /* check if limit reached */
            break;
    }

    free (cpy);     /* free copy */

    return inputs;
}

void mainProcess()
{    
    char *input = NULL,
        **words = NULL;
    size_t len = 0, nwords = 0;

    printf ("Inside of Main process\n\n");

    input = getInput (&len);

    if (!input || !*input) {
        fprintf (stderr, "error: input is empty or NULL.\n");
        return;
    }

    printf ("this is input '%s' with len: %zu (before split)\n", input, len);

    words = splitInput (input, &nwords);

    printf ("this is input '%s' with len: %zu (after split)\n", input, len);

    free (input);  /* done with input, free it! */

    printf ("the words in input are:\n");

    for (size_t i = 0; i < nwords; i++) {
        printf ("  word[%2zu]: '%s'\n", i, words[i]);
        free (words[i]);    /* free each word */
    }
    free (words);           /* free pointers */

    putchar ('\n');     /* tidy up with newline */
}

Example Use/Output

$ ./bin/mainprocess
Inside of Main process

Inside of the getInput
$ my dog has fleas
my dog has fleas
this is input 'my dog has fleas' with len: 16 (before split)
inside split
my dog has fleas
this is input 'my dog has fleas' with len: 16 (after split)
the words in input are:
  word[ 0]: 'my'
  word[ 1]: 'dog'
  word[ 2]: 'has'
  word[ 3]: 'fleas'

End of the program

Memory Error Check

In any code you write that dynamically allocates memory, you need to run your code though a memory/error checking program. On Linux, valgrind is the normal choice. Simply run your code through it, e.g.

$ valgrind ./bin/mainprocess
==15900== Memcheck, a memory error detector
==15900== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==15900== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==15900== Command: ./bin/mainprocess
==15900==
Inside of Main process

Inside of the getInput
$ my dog has fleas
my dog has fleas
this is input 'my dog has fleas' with len: 16 (before split)
inside split
my dog has fleas
this is input 'my dog has fleas' with len: 16 (after split)
the words in input are:
  word[ 0]: 'my'
  word[ 1]: 'dog'
  word[ 2]: 'has'
  word[ 3]: 'fleas'

End of the program
==15900==
==15900== HEAP SUMMARY:
==15900==     in use at exit: 0 bytes in 0 blocks
==15900==   total heap usage: 7 allocs, 7 frees, 546 bytes allocated
==15900==
==15900== All heap blocks were freed -- no leaks are possible
==15900==
==15900== For counts of detected and suppressed errors, rerun with: -v
==15900== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always verify you have freed any memory you allocate, and that there are no memory errors.

Look things over and let me know if you have any questions. If I guess wrong about what you intended, well that's where an MCVE helps :)

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • Sure, glad you got it sorted out. If you don't actually need to split things up into the separate functions, you can reduce the code to about 25 lines total. However, by splitting it up, it is a good exercise in parameter and return handling. Good luck with your coding. – David C. Rankin Apr 18 '17 at 19:39
1

This code compiles (gcc -Wall) without warnings and does not change the size.
It also tries to stress the need for allocating enough space and/or not to write beyond allocated memory.
Note for example the

  • malloc((MaxInputLength+1) * sizeof(char))
  • while(length<MaxInputLength)
  • inputs[i]=malloc((MaxLengthOfSplitString+1) * sizeof(char));

    #include <stdio.h>  
    #include <stdlib.h>
    #include <string.h>
    
    // the length which was used in your MCVE, probably accidentally  
    #define MaxInputLength 3           // you will probably want to increase this
    #define MaxLengthOfSplitString  1  // and this
    #define MaxNumberOfSplitStrings 3  // and this
    
    //  Reading the input from the user
    char * getInput(){
        printf("Inside of the getInput\n");
        char * result;
        char c;
        int length=0;
        result = malloc((MaxInputLength+1) * sizeof(char));
    
        // code goes here 
        printf("$ ");
        while(length<MaxInputLength){
            c = fgetc(stdin);
            if(c == 10){
                break;
            }
            printf("%c", c);
            result[length] = c;
            length++;
        }   
    
        result[length] = '\0'; 
    
        return result;
    }
    
    char ** splitInput(const char * str){
        char ** inputs;
        inputs = NULL;
        printf("inside split\n");
        printf("%s\n", str);
    
        inputs = (char **) malloc(MaxNumberOfSplitStrings * sizeof(char*));
        {
            int i;
            for (i=0; i< MaxNumberOfSplitStrings; i++)
            {
                inputs[i]=malloc((MaxLengthOfSplitString+1) * sizeof(char));
            }
    
            // Now you have an array of MaxNumberOfSplitStrings char*.
            // Each of them points to a buffer which can hold a ero-    terminated string
            // with at most MaxLengthOfSplitString chars, ot counting the     '\0'.
        }
    
        // free(inputs);
        printf("------\n"); // for testing
        printf("%s\n", str);
        if(!inputs){
            printf("Error in initializing the 2D array!\n");
            exit(EXIT_FAILURE);
        }
    
        return NULL;
    }
    
    void mainProcess(){
        char * input;
        printf("Inside of Main process\n");
        input = getInput();
        printf("\nthis is input %s with len  %d\n", input, strlen(input));
        splitInput(input);
        printf("\nthis is input %s with len  %d\n", input, strlen(input));
    }
    
    int main(int argc, char const *argv[])
    {
        /* code */
        mainProcess();
        printf("\nEnd of the program\n");
        return 0;
    }
    
Yunnosch
  • 26,130
  • 9
  • 42
  • 54