Error using malloc

Question

I pass char ** input from main() to processInExp() function, then I pass it again from processInExp() function to getInput() function to dynamically allocate it over while reading through the file.

Inside getInput() function input is allocated memory properly when checked, but while using it in in processInExp() it encounters gets runtime error. What can be the issue?

Below is my code:

int getInput(char ** input, const char * fileName)
{
    int numInput = 0;
    int i, j;
    char c;
    char tempInput[100];
    FILE * pFile;
    if((pFile = fopen(fileName, "r")) == NULL)
    {
        printf("Cannot read file %s\n", fileName);
        system("PAUSE");
        exit(1);
    }
    while(!feof(pFile))
    {
        c = fgetc(pFile);
        if(c == '\n') ++numInput;

    }
    /* printf("%d\n", numInput); */
    input = (char**)malloc(numInput * sizeof(char*)); /* #2 MALLOC input */
    rewind(pFile);
    for(i = 0; !feof(pFile); ++i)
    {
        fscanf(pFile, "%[^\n]%*c", tempInput);
        /* printf("%s\n", tempInput); */
        input[i] = (char*)malloc((strlen(tempInput) + 1) * sizeof(char)); /* #3 MALLOC input[] */
        strcpy(input[i], tempInput);
        /* printf("%s\n", input[i]); */ /* #4 PRINT OUT PERFECTLY */
        memset(tempInput, 0, sizeof(tempInput));
    }
    fclose(pFile);
    return numInput;
}
void processInExp(char ** input, char ** output, const char * fileName)
{
    int numFormula;
    int i;

    numFormula = getInput(input, fileName); /* #1 PASSING input */
    /* printf("%s\n", input[0]); */ /* #5 RUNTIME ERROR */
    output = (char**)malloc(numFormula * sizeof(char*));
    system("PAUSE");

    for(i = 0; i < numFormula; ++i)
    {
        convertIntoPost(input[i], output[i]);
        printf("%d. %s -> %s", (i + 1), input[i], output[i]);
    }

}

Answer 1

While others have pointed out the issue with pass by value, there is another issue where learning can occur. There is no need to pre-read the file to determine the number of characters or lines and then rewind the file to read each line.

Take a look at getline which returns the number of characters read. All you need to do is keep a sum variable and after reading all line, simply return (or update a pointer you provided as an argument) and you are done. Of course you can do the same with fscanf or fgets by calling strlen after reading the line.

The following is a short example of reading a text file in one pass while determining the number of characters (without the newline) and returning that information to the calling function. Just as you needed to pass a pointer to your array of pointers in getInput, we will use pointers passed as arguments to return the line and character counts to our calling function. If you declare and call the function to read the file as follows:

size_t nline = 0;       /* placeholders to be filled by readtxtfile */
size_t nchar = 0;       /* containing number of lines/chars in file */
...
char **file = readtxtfile (fn, &nline, &nchar);

By declaring the variables in the calling function, and then passing pointers to the variables as arguments (using the urnary &), you can update the values in the function and have those values available for use back in main (or whatever function you called readtxtfile from.)

A quick example illustrating these points could be:

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

#define NMAX 256

char **readtxtfile (char *fn, size_t *idx, size_t *sum);
void prn_chararray (char **ca);
void free_chararray (char **ca);

int main (int argc, char **argv) {

    size_t nline = 0;       /* placeholders to be filled by readtxtfile */
    size_t nchar = 0;       /* containing number of lines/chars in file */
    char *fn = argc > 1 ? argv[1] : NULL;/* if fn not given, read stdin */

    /* read each file into an array of strings,
     * number of lines/chars read updated in nline, nchar
     */
    char **file = readtxtfile (fn, &nline, &nchar);

    /* output number of lines read & chars read and from where  */
    printf ("\n read '%zu' lines & '%zu' chars from file: %s\n\n", 
            nline, nchar, fn ? fn : "stdin");

    /* simple print function to print all lines */
    if (file) prn_chararray (file);

    /* simple free memory function */
    if (file) free_chararray (file);

    return 0;
}

/* simple function using getline to read any text file and return
 * the lines read in an array of pointers. user is responsible for
 * freeing memory when no longer needed
 */
char **readtxtfile (char *fn, size_t *idx, size_t *sum)
{
    char *ln = NULL;                /* NULL forces getline to allocate  */
    size_t n = 0;                   /* line buf size (0 - use default)  */
    ssize_t nchr = 0;               /* number of chars actually read    */
    size_t nmax = NMAX;             /* check for reallocation           */
    char **array = NULL;            /* array to hold lines read         */
    FILE *fp = NULL;                /* file pointer to open file fn     */

    /* open / validate file or read stdin */
    fp = fn ? fopen (fn, "r") : stdin;
    if (!fp) {
        fprintf (stderr, "%s() error: file open failed '%s'.", __func__, fn);
        return NULL;
    }

    /* allocate NMAX pointers to char* */
    if (!(array = calloc (NMAX, sizeof *array))) {
        fprintf (stderr, "%s() error: memory allocation failed.", __func__);
        return NULL;
    }

    /* read each line from stdin - dynamicallly allocated   */
    while ((nchr = getline (&ln, &n, fp)) != -1)
    {
        /* strip newline or carriage rtn    */
        while (nchr > 0 && (ln[nchr-1] == '\n' || ln[nchr-1] == '\r'))
            ln[--nchr] = 0;

        *sum += nchr;               /* add chars in line to sum         */

        array[*idx] = strdup (ln);  /* allocate/copy ln to array        */

        (*idx)++;                   /* increment value at index         */

        if (*idx == nmax) {         /* if lines exceed nmax, reallocate */
            char **tmp = realloc (array, nmax * 2);
            if (!tmp) {
                fprintf (stderr, "%s() error: reallocation failed.\n", __func__);
                exit (EXIT_FAILURE); /* or return NULL; */
            }
            array = tmp;
            nmax *= 2;
        }
    }

    if (ln) free (ln);              /* free memory allocated by getline */
    if (fp != stdin) fclose (fp);   /* close open file descriptor       */

    return array;
}

/* print an array of character pointers. */
void prn_chararray (char **ca)
{
    register size_t n = 0;
    while (ca[n])
    {
        printf (" arr[%3zu]  %s\n", n, ca[n]);
        n++;
    }
}

/* free array of char* */
void free_chararray (char **ca)
{
    if (!ca) return;
    register size_t n = 0;
    while (ca[n])
        free (ca[n++]);
    free (ca);
}

Use/Output

$ ./bin/getline_ccount <dat/fc-list-fonts.txt

 read '187' lines & '7476' chars from file: stdin

 arr[  0]  andalemo.ttf: Andale Mono - Regular
 arr[  1]  arialbd.ttf: Arial - Bold
 arr[  2]  arialbi.ttf: Arial - Bold Italic
 arr[  3]  ariali.ttf: Arial - Italic
 arr[  4]  arialnbi.ttf: Arial
 arr[  5]  arialnb.ttf: Arial
 arr[  6]  arialni.ttf: Arial
 arr[  7]  arialn.ttf: Arial
 arr[  8]  arial.ttf: Arial - Regular
 arr[  9]  ARIALUNI.TTF: Arial Unicode MS - Regular
 arr[ 10]  ariblk.ttf: Arial
 arr[ 11]  Bailey Script Regular.ttf: Bailey Script - Regular
 arr[ 12]  Bailey_Script_Regular.ttf: Bailey Script - Regular
 arr[ 13]  Belwe Gotisch.ttf: Belwe Gotisch - Regular
 arr[ 14]  Belwe_Gotisch.ttf: Belwe Gotisch - Regular
 <snip>

Memory/Leak Check

Whenever you allocated/free memory in your code, don't forget to use a memory checker to insure there are no memory errors or leaks in your code:

$ valgrind ./bin/getline_ccount <dat/fc-list-fonts.txt
==20259== Memcheck, a memory error detector
==20259== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==20259== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==20259== Command: ./bin/getline_readfile_function
==20259==

 read '187' line from file: stdin

 arr[  0]  andalemo.ttf: Andale Mono - Regular
 arr[  1]  arialbd.ttf: Arial - Bold
 arr[  2]  arialbi.ttf: Arial - Bold Italic
 arr[  3]  ariali.ttf: Arial - Italic
<snip>

==20259==
==20259== HEAP SUMMARY:
==20259==     in use at exit: 0 bytes in 0 blocks
==20259==   total heap usage: 189 allocs, 189 frees, 9,831 bytes allocated
==20259==
==20259== All heap blocks were freed -- no leaks are possible
==20259==
==20259== For counts of detected and suppressed errors, rerun with: -v
==20259== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)

---

Follow On From Comment

There are several issues with the code you posted in the comment:

for(i = 0; !feof(pFile); ++i) {
    fscanf(pFile, "%[^\n]%*c", tempInput); 
    /* printf("%s\n", tempInput); */ 
    input[i] = (char*)malloc((strlen(tempInput) + 1) * sizeof(char)); 
    strcpy(input[i], tempInput); 
    printf("%s\n", input[i]); 
    memset(tempInput, 0, sizeof(tempInput)); 
} 
    for(i = 0; i < numInput; ++i) {
    convertIntoPost(input[i], output[i]);
}

First, read the link in the first comment about why feof can cause problems when using it to indicate EOF in a loop. Second, functions have return values, the ability to use them for an advantage tells you whether you are using the correct function for the job.

The difficulty you are having trying to shoehorn reading an entire line with fscanf should be telling you something... The problem you have backed into by your choice of the format specifier "%[^\n]%*c" to read a line containing whitespace is the exact reason fscanf is NOT the proper tool for the job.

Why? The scanf family of functions were created to read discrete values. Their return is based on:

the number of input items successfully matched and assigned

Using your format specifier, the number of items read on success is 1. The *%c reads and discards the newline, but is NOT added to the item count. This causes a BIG problem when trying to read a file that can contain blank lines. What happens then? You experience an input failure and fscanf returns 0 -- but it is still very much a valid line. When that occurs, nothing is read. You cannot check the return as being >= 0 because when you encounter a blank line you loop forever...

With your format specifier, you cannot check for EOF either. Why? With the scanf family of functions:

The value EOF is returned if the end of input is reached before either the first successful conversion or a matching failure occurs.

That will never occur in your case because you have an input failure with fscanf (not end of input) and no matching failure has occurred. Are you starting to see why fscanf may not be the right tool for the job?

The C library provides two functions for line-oriented input. They are fgets and getline. Both read an entire line of text into the line buffer. This will include the newline at the end of each line (including blank lines). So when you use either to read text, it is a good idea to remove the newline by overwriting with a null-terminating character.

Which to use? With fgets, you can limit the number of characters read by sizing the character buffer appropriately. getline is now part of the C library, and it provides the added benefit of returning the number of characters actually read (a bonus), but it will read the line no matter how long it is because it dynamically allocates the buffer for you. I prefer it, but just know that you need to check the number of characters it has read.

Since I provided a getline example above, your read loop can be much better written with fgets as follows:

    while (fgets (tempInput, MAXL, pFile) != NULL) {
        nchr = strlen (tempInput);
        while (nchr && (tempInput[nchr-1] == '\n' || tempInput[nchr-1] == '\r'))
            tempInput[--nchr] = 0;     /* strip newlines & carriage returns */
        input[i++] = strdup (tempInput);    /* allocates & copies tempInput */
    }
    numInput = i;

Next, your allocation does not need to be cast to (char *). The return of malloc and calloc is just a pointer to (i.e. the address of) the block of memory allocated. (it is the same no matter what you are allocating memory for) There is no need for sizeof (char). It is always 1. So just write:

input[i] = malloc (strlen(tempInput) + 1); 
strcpy (input[i], tempInput);

A more convenient way to both allocate and copy is using strdup. With strdup, the two lines above become simply:

input[i++] = strdup (tempInput);    /* allocates & copies */

Next, there is no need for memset.

memset(tempInput, 0, sizeof(tempInput));

If tempInput is declared to hold 100 chars with say: tempInput[100], you can read strings up to 99 char into the same buffer over-and-over again without ever having to zero the memory. Why? Stings are null-terminated. You don't care what is in the buffer after the null-terminator...

That's a lot to take in. Putting it all together in a short example, you could do something like:

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

#define MAXL 256

/* dummy function */
void convertIntoPost (char *in, char **out)
{
    size_t i = 0, len = strlen (in);
    *out = calloc (1, len + 1);

    for (i = 0; i < len; i++) {
        (*out)[len-i-1] = in[i];
    }
}

int main (int argc, char **argv) {

    char tempInput[MAXL] = {0};
    char **input = NULL, **output = NULL;
    size_t i = 0, numInput = 0;
    size_t nchr = 0;
    FILE *pFile = NULL;

    pFile = argc > 1 ? fopen (argv[1], "r") : stdin;

    if (!pFile) {
        fprintf (stderr, "error: file open failed '%s'.\n", 
                argv[1] ? argv[1] : "stdin");
        return 1;
    }

    input  = calloc (1, MAXL);  /* allocate MAXL pointer for input & output   */
    output = calloc (1, MAXL);  /* calloc allocates and sets memory to 0-NULL */

    if (!input || !output) {    /* validate allocation */
        fprintf (stderr, "error: memory allocation failed.\n");
        return 1;
    }

    while (fgets (tempInput, MAXL, pFile) != NULL) {
        nchr = strlen (tempInput);
        while (nchr && (tempInput[nchr-1] == '\n' || tempInput[nchr-1] == '\r'))
            tempInput[--nchr] = 0;
        input[i++] = strdup (tempInput);    /* allocates & copies */
    }
    numInput = i;

    fclose (pFile);

    /* call convertIntoPost with input[i] and &output[i] */
    for (i = 0; i < numInput; ++i) {
        convertIntoPost (input[i], &output[i]);
        printf (" input[%2zu]: %-25s  output[%2zu]: %s\n",
                i, input[i], i, output[i]);
    }

    /* free all memory */
    for (i = 0; i < numInput; ++i) {
        free (input[i]), free (output[i]);
    }
    free (input), free (output);

    return 0;
}

Example Output

$ ./bin/feoffix ../dat/captnjack.txt
 input[ 0]: This is a tale             output[ 0]: elat a si sihT
 input[ 1]: Of Captain Jack Sparrow    output[ 1]: worrapS kcaJ niatpaC fO
 input[ 2]: A Pirate So Brave          output[ 2]: evarB oS etariP A
 input[ 3]: On the Seven Seas.         output[ 3]: .saeS neveS eht nO

Notes On Compiling Your Code

Always compile your code with Warnings enabled. That way the compiler can help point out areas where your code may have ambiguity, etc.. To enable warnings when you compile, simply add -Wall and -Wextra to your compile string. (If you really want all warnings, add -pedantic (definition: overly concerned with trivial details)). The take the time to read and understand what the compiler is telling you with the warnings (they are really very good, and you will quickly learn what each means). Then... go fix the problems so your code compiles without any warnings.

There are only very rare and limited circumstances where it is permissible to 'understand and choose to allow' a warning to remain (like when using a library where you have no access to the source code)

So putting it all together, when you compile your code, at a minimum you should be compiling with the following for testing and development:

gcc -Wall -Wextra -o progname progname.c -g

With gcc, the -g option tell the compiler to produce additional debugging information for use with the debugger gdb (learn it).

When you have all the bugs worked out and you are ready for a final compile of your code, you will want to add optimizations like the optimization level -On (that's capital O [not zero] where 'n' is the level 1, 2, or 3 (0 is default), -Ofast is essentially -O3 with a few additional optimizations). You may also want to consider telling the compiler to inline your functions when possible with -finline-functions to eliminate function call overhead. So for final compile you will want something similar to:

gcc -Wall -Wextra -finline-functions -Ofast -o progname progname.c

The optimizations can produce a 10-fold increase in performance and decrease in your program execution time (that's a 1000% increase in performance in some cases (300-500% improvement is common)). Well worth adding a couple of switches.

Answer 2

C uses pass-by-value for function argument passing. So, from inside the function getInput(), you cannot change the variable input and expect that change to be reflected back in the actual argument, passed to the function. For that, you'll need a pointer-to variable to be passed, like in this case, you need to do

   int getInput(char *** input, const char * fileName) { //notice the extra *

and need to call it like

   char ** inp = NULL;
   getInput(&inp, ..........);

Then, getInput() will be able to allocate memory to *input inside the function which will be reflected into inp.

Otherwise, after returning from the getInput(), the actual argument will still be uninitialized and using that further (in your case, in the for loop in processInExp() function) will lead to undefined behaviour.

That said, two more important things to notice,

1. Please see why not to cast the return value of malloc() and family in C.
2. Check Why is while ( !feof (file) ) always wrong?

Answer 3

As Sourav mentioned, C uses pass-by-value for argument passing, so the input variable within the scope of processInExp has the value of the address of the memory previously allocated in main.

This results in a segmentation fault when you print input[0]. This is because printf is trying to print the string located at the address relative to the previously allocated memory instead of memory allocated to input in the getInput function to which you copied the string.

A solution would be to pass a pointer to input, so your function signature would like like this: int getInput(char *** input, const char * fileName). You would then need to change any references to input to *input in order to dereference the pointer, and pass input's pointer to getInput like this: getInput(&input, fileName).

Answer 4

The C language is pass-by-value without exception.

A function is not able to change the value of actual parameters.