0

i wanted to create a dynamic array, which will contain user input. But I keep getting segmentation fault as an error after my first input. I know that segmentation fault is caused due false memory access. Is there a way to locate the error in the code ?

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

int main(int argc, char *argv[])
 {
    int length,i;
    int size_arr = 1;
    int size_word = 102;
    char **arr;
    char *word;
    arr = malloc(size_arr * sizeof(char*));
    word = malloc(size_word *sizeof(char));
    if(arr == NULL)
    {
        fprintf(stderr, "No reallocation");
        exit(EXIT_FAILURE);
    }
    else
    {
        while(!feof(stdin) && fgets(word, size_word , stdin) != NULL)
        {   
            arr[i] = strndup(word,size_word);
            length = strlen(word);
            if(length > 102)
            {
                fprintf(stderr, "Word is too long");
            }
            if(size_arr-1 == i)
            {
                size_arr *= 2;
                arr = realloc(arr, size_arr * sizeof(char*));
            }
            i++;
        }
 
    }
    free(arr);
    return 0;
}

Greeting, Soreseth

Soreseth
  • 3
  • 1
  • 1
    Initialize 'i' with 0 – tstanisl Jun 01 '21 at 17:26
  • 1
    You will want to look at [Why is while ( !feof (file) ) always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) .. Just get rid of it. Control your read-loop with the return from `fgets()` only. `strndup(word,size_word)` allocates -- you must VALIDATE, just as if you had called `malloc()` directly. Never `realloc()` to the pointer itself. When `realloc()` fails you overwrite your pointer with `NULL`. Use a temp pointer, e.g. `void *tmp = realloc(arr, size_arr * sizeof(char*)); if (!tmp) {/* handle error */} arr = tmp;` – David C. Rankin Jun 01 '21 at 17:28
  • Hello, I'm not able to reproduce your issue. I'm not seeing SegFault when i run your code. If you set `i=0` while declaring it, does it fix your issue? – Rohan Kumar Jun 01 '21 at 17:56
  • Thanks for the advice @DavidC.Rankin I'll try it out:) – Soreseth Jun 01 '21 at 17:56
  • Setting i to 0 dont fix the issue :( @RohanKumar – Soreseth Jun 01 '21 at 17:58
  • What error are you getting exactly? What Input did you provide? – Rohan Kumar Jun 01 '21 at 17:59
  • You should try debugging your program in gdb to see which line is causing SegFault – Rohan Kumar Jun 01 '21 at 18:00

1 Answers1

1

You are close, you are just a bit confused on how to handle putting all the pieces together. To begin with, understand there are no arrays involved in your code. When you are building a collection using a pointer-to-pointer (e.g. char **arr), it is a two step process. arr itself is a single-pointer to an allocated block of pointers -- which you expand by one each time to add the next word by calling realloc() to reallocate an additional pointer.

The second step is to allocated storage for each word. You then assign the beginning address for that word's storage to the pointer you have allocated.

So you have one block of pointers which you expand (realloc()) to add a pointer to hold the address for the next word, you then allocate storage for that word, assigning the beginning address to your new pointer and then copy the word to that address.

(note: calling realloc() every iteration to add just one-pointer is inefficient. You solve that by adding another counter (e.g. allocated) that holds the number of pointers allocated, and a counter used, to keep track of the number of pointers you have used. You only reallocate when used == allocated. That way you can, e.g. double the number of pointers available each time -- or whatever growth scheme you choose)

Also note that strdup() and strndup() are handy, but are not part of the standard C library (they are POSIX). While most compilers will provide them, you may need the right compiler option to ensure they are available.

Let's look at your example, in the simple case, using only functions provided by the standard library. We will keep you reallocate by one scheme, and leave you to implement the used == allocated scheme to clean that up later.

When reading lines of data, you won't know how many characters you need to store until the line is read -- so just reused the same fixed-size buffer to read each line. Then you can trim the '\n' included by fgets() and get the length of the characters you need to allocate (+1 for the *nul-terminating character). Then simply allocate, assign to your new pointer and copy from the fixed buffer to the new block of storage for the word (or line). A 1K, 2K, etc.. fixed buffer is fine.

So let's collect the variables you need for your code, defining a constant for the fixed buffer size, e.g.

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

#define MAXC 1024       /* if you need a constant, #define one (or more) */

int main (void)
{
    char **arr = NULL,      /* pointer to block of allocated pointers */
        buf[MAXC];          /* fixed buffer for read-input */
    int size_arr = 0;       /* only 1 counter needed here */

Now let's read a line into buf and start by allocating your pointer:

    while (fgets (buf, MAXC, stdin))
    {   
        size_t len;
        
        /* allocate pointer (one each time rather inefficient) */
        void *tmp = realloc (arr, (size_arr + 1) * sizeof *arr);
        if (!tmp) {                                 /* VALIDATE */
            perror ("realloc-arr");
            break;
        }
        arr = tmp;                                  /* assign on success */

(as noted in my comment, you never realloc() using the pointer itself because when (not if) realloc() fails, it will overwrite the pointer address (e.g. the address to your collection of pointers) with NULL.)

So above, you realloc() to the temporary pointer tmp, validate the reallocation succeeded, then assign the newly allocated block of pointers to arr.

Now trim the '\n' from buf and get the number of characters. (where strcspn() allows you to do this all in a single call):

        buf[(len = strcspn (buf, "\n"))] = 0;       /* trim \n, save len */

Now just allocated storage for len + 1 characters and copy from buf to arr[size_arr].

        arr[size_arr] = malloc (len + 1);           /* allocate for word */
        if (!arr[size_arr]) {                       /* VALIDATE */
            perror ("malloc-arr[i]");
            break;
        }
        memcpy (arr[size_arr], buf, len + 1);       /* copy buf to arr[i] */
        size_arr += 1;                              /* increment counter */
    }

(note: when reallocating 1 pointer per-iteration, only a single counter variable is needed, and note how it is not incremented until both the pointer reallocation, the allocation for your word storage is validated, and the word is copied from buf to arr[size_arr]. On failure of either allocation, the loop is broken and your size_arr will still hold the correct number of stored words)

That completes you read-loop.

Now you can use your stored collection of size_arr pointers, each pointing to an allocated and stored word as you wish. But remember, when it comes time to free the memory, that too is a 2-step process. You must free the allocated block for each word, before freeing the block of allocated pointers, e.g.

     for (int i = 0; i < size_arr; i++) {            /* output result */
        puts (arr[i]);
        free (arr[i]);                              /* free word storage */
    }
    free(arr);                                      /* free pointers */

Done.

The complete program is:

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

#define MAXC 1024       /* if you need a constant, #define one (or more) */

int main (void)
{
    char **arr = NULL,      /* pointer to block of allocated pointers */
        buf[MAXC];          /* fixed buffer for read-input */
    int size_arr = 0;       /* only 1 counter needed here */
    
    while (fgets (buf, MAXC, stdin))
    {   
        size_t len;
        
        /* allocate pointer (one each time rather inefficient) */
        void *tmp = realloc (arr, (size_arr + 1) * sizeof *arr);
        if (!tmp) {                                 /* VALIDATE */
            perror ("realloc-arr");
            break;
        }
        arr = tmp;                                  /* assign on success */
        
        buf[(len = strcspn (buf, "\n"))] = 0;       /* trim \n, save len */
        
        arr[size_arr] = malloc (len + 1);           /* allocate for word */
        if (!arr[size_arr]) {                       /* VALIDATE */
            perror ("malloc-arr[i]");
            break;
        }
        memcpy (arr[size_arr], buf, len + 1);       /* copy buf to arr[i] */
        size_arr += 1;                              /* increment counter */
    }
    
    for (int i = 0; i < size_arr; i++) {            /* output result */
        puts (arr[i]);
        free (arr[i]);                              /* free word storage */
    }
    free(arr);                                      /* free pointers */
}

Example Use/Output

Test it out. Do something creative like read, store and output your source file to make sure it works, e.g.

$ ./bin/dynarr < dynarr.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAXC 1024       /* if you need a constant, #define one (or more) */

int main (void)
{
    char **arr = NULL,      /* pointer to block of allocated pointers */
        buf[MAXC];          /* fixed buffer for read-input */
    int size_arr = 0;       /* only 1 counter needed here */

    while (fgets (buf, MAXC, stdin))
    {
        size_t len;

        /* allocate pointer (one each time rather inefficient) */
        void *tmp = realloc (arr, (size_arr + 1) * sizeof *arr);
        if (!tmp) {                                 /* VALIDATE */
            perror ("realloc-arr");
            break;
        }
        arr = tmp;                                  /* assign on success */

        buf[(len = strcspn (buf, "\n"))] = 0;       /* trim \n, save len */

        arr[size_arr] = malloc (len + 1);           /* allocate for word */
        if (!arr[size_arr]) {                       /* VALIDATE */
            perror ("malloc-arr[i]");
            break;
        }
        memcpy (arr[size_arr], buf, len + 1);       /* copy buf to arr[i] */
        size_arr += 1;                              /* increment counter */
    }

    for (int i = 0; i < size_arr; i++) {            /* output result */
        puts (arr[i]);
        free (arr[i]);                              /* free word storage */
    }
    free(arr);                                      /* free pointers */
}

Memory Use/Error Check

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

$ valgrind ./bin/dynarr < dynarr.c
==30995== Memcheck, a memory error detector
==30995== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==30995== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==30995== Command: ./bin/dynarr
==30995==
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

    <snipped code>
}
==30995==
==30995== HEAP SUMMARY:
==30995==     in use at exit: 0 bytes in 0 blocks
==30995==   total heap usage: 84 allocs, 84 frees, 13,462 bytes allocated
==30995==
==30995== All heap blocks were freed -- no leaks are possible
==30995==
==30995== For counts of detected and suppressed errors, rerun with: -v
==30995== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm that you have freed all memory you have allocated and that there are no memory errors.

Look things over and let me know if you have further questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85