-1

In C, I want to fill a dynamic array with each character of a file in each of his boxes. But instead, when I print my array I have:

0 = [] 
1 = [] 
2 = [] 
3 = [] 
4 = [] 
5 = [] 
6 = [] 
7 = [] 
8 = [] 
9 = []

I have no compilation errors, but valgrind said that I have a:

Conditional jump or move depends on uninitialised value(s) 

in my main at the printf. That's strange, because even with an initialisation in my main:

array_valid_char = NULL;

valgrind keeps getting me that error. And even if I change to:

printf("%d = [%d] \n", i, array_valid_char[i]);

the display is the same.

Here is my code:

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

int* f(char* name_file_in)
{
    FILE *file_in;
    int* array_valid_char = malloc(10 * sizeof(int*));
    int read_char;
    file_in = fopen(name_file_in,"rb");
    if(file_in)
    {
        while ((read_char = fgetc(file_in)) != EOF)
        {
            *(array_valid_char++) = read_char;
        }
    }
    if(file_in){fclose(file_in);}
    return array_valid_char;
}

int main(int argc,char* argv[])
{
    int *array_valid_char = malloc(10 * sizeof(int*));
    array_valid_char = f(argv[1]);
    for (int i = 0; i < 10; i++)
    {
        printf("%d = [%c] \n", i, array_valid_char[i]);
    }
    return(0);
}

What is wrong with my code?

  • Consider what happens if "if my file is: abcdefghik" is only the beginning and in addition there was a following `'\n'`. Allocated memory assigned to `array_valid_char` overflows leading to _undefined behavior_ (UB). – chux - Reinstate Monica Jan 11 '18 at 19:52
  • @user3121023 then `int* array_valid_char` is not the right type. Allocation should match the `ptr = mallloc(sizeof *ptr * n)` model. – chux - Reinstate Monica Jan 11 '18 at 19:53
  • @user3121023.: Yes – user2736738 Jan 11 '18 at 19:57
  • @user3121023 [sizeof *ptr in your example would be int](https://stackoverflow.com/questions/48214739/why-dynamic-1d-array-dont-want-to-input-each-character-of-a-file-in-c?noredirect=1#comment83411523_48214739) `sizeof *ptr` returns a `size_t`, not an `int`. The value, in this case would be the same as `sizeof (int)`. – chux - Reinstate Monica Jan 11 '18 at 20:02

2 Answers2

5

You have to keep track of the beginning of the allocated memory which you didn't. (That original allocated chunks address should be returned ).

int* array_valid_char = malloc(10 * sizeof(int*));
int *st = array_valid_char ;

...
return st;

Also you have memory leak - you can omit the malloc in main().

Also you need to free the dynamically allocated memory when you are done working with it.

free(array_valid_char);

And also the memory allocation part would be

int* array_valid_char = malloc(10 * sizeof(int));

or

 int* array_valid_char = malloc(10 * sizeof(*array_valid_char));

You want array of int.

Among other points check the return value of malloc and handle it properly.

The correct way to code would be to index into the allocated memory and check whether we are reaching limit of what we allocated - if it is so then reallocate.

Many of us confine ourselves with the thought that sizeof( *ptr)*10 is only good enough for the clear syntax etc but knowing that sizeof returns size_t when multiplied this with other value it is less likely to overflow as in opposed to writing the other way round(which was having int arithmetic) that's a benefit. (chux) For example: sizeof(something)*int*int will do result in operation with size_t value which will less likely to overflow that int*int*sizeof(int). In second case int*int may overflow.(more likely)

user2736738
  • 28,590
  • 4
  • 37
  • 52
  • @yano Note: Leading with `sizeof *array_valid_char` has advantages when `10` is more complex and large like `sizeof *array_valid_char * 100000 * 100000` as it insures all the math is done with at least `size_t` math and not `int` math - which may overflow. – chux - Reinstate Monica Jan 11 '18 at 19:59
  • @chux oh so you're saying `10 * sizeof(*array_valid_char)` performs integer math whereas `sizeof(*array_valid_char) * 10` performs `size_t` math? – yano Jan 11 '18 at 20:05
  • correct me if I'm wrong, the free function must be in my main. Not in my function f. Right? – Flavien Chamay Jan 11 '18 at 20:06
  • @LinuxCrusher.: Yep – user2736738 Jan 11 '18 at 20:07
  • @yano. No. `10 * sizeof(*array_valid_char)` will convert `10` to `size_t` and do the math as `size_t * size_t`. No benefit here leading with `10` or `sizeof`. It is with `int*int*size_t` versus `size_t*int*int` where leading with `sizeof` benefits. The first can do a `int*int` which may OF, but maybe not if it was `size_t` math. – chux - Reinstate Monica Jan 11 '18 at 20:07
  • @chux.: I guess that is because `*` is left associative - so `int` arithemtic may overflow. – user2736738 Jan 11 '18 at 20:09
  • Yes, The general idea is simple: with math, often good to insure the math is done with the widest type of the arguments [and the destination](https://stackoverflow.com/a/40637622/2410359) – chux - Reinstate Monica Jan 11 '18 at 20:13
  • @chux.: Ah read that answer before. Nice one..yes I get the point. – user2736738 Jan 11 '18 at 20:15
  • @chux.: Thanks again..a lot. – user2736738 Jan 11 '18 at 20:17
1

There are some problems in your code:

With *(array_valid_char++), you move your pointer each time you pass on the loop. If you want to use this method, you need to keep a track on the beginning of your array with an other variable. You can also use an iterator array_valid_char[i] that starts at 0 and increments it at each loop turn.

In your main, you malloc your array int *array_valid_char = malloc(10 * sizeof(int*)); but you override it just after with array_valid_char = f(argv[1]);. If you malloc your array in a function and send it back with a return, the memory is still allocated.

In printf, %d is for display a number and %c is for display a character. In your case, you need to use %c. In the other case, you will see the ASCII value of your character.

By the way, you also use an int array to receive char array. It is not a problem now but for some optimisation, you can use char array to take less memory.

Also, don't forget to free the memory you have used when you don't use it anymore, it could be useful in bigger programs.

chux - Reinstate Monica
  • 113,725
  • 11
  • 107
  • 213
  • I just came up with a solution but with *(array_valid_char++). It is functionning well. Or maybe that's not an optimize way to code? – Flavien Chamay Jan 11 '18 at 20:23
  • @LinuxCrusher *(array_valid_char++) is good. The idea was just to explain you the difference between the two methods. – Gaetan Leandre Jan 11 '18 at 20:34