0

This is simple, I am allocating a dynamic 2d array using functions. I limited the scanf() len and my problem is when input a value over the limit, something weird happen.

Example
Input: 111,222,333,444
Expected output: 11,22,33,44
Real output: 11,12,33,34

#include <stdio.h>
#include <stdlib.h>
#define gd 2

void get_mem(int ***arr);
void get_data(int **arr);

int main(){
    int **arr;
    arr = NULL;
    get_mem(&arr);
    get_data(arr);
    free(*arr);
    return 0;
}

void get_mem(int ***arr){
    int i;
    *arr =  (int**)malloc(gd*sizeof(int*));

for(i=0;i<5;i++){
    (*arr)[i] = (int*)malloc(gd*sizeof(int));
}
printf("oki\n");
}

void get_data(int **arr){
    int c,f;
    for(c=0;c<gd;c++){
        for(f=0;f<gd;f++){
            scanf("%2d",&*(*arr+c)+f);
            fpurge(stdin);
            fflush(stdin);          
        }
    }    
for(c=0;c<gd;c++){
    for(f=0;f<gd;f++){
        printf("%d ",*(*arr+c)+f);
        printf("\n");
    }

  }
}
SayAz
  • 751
  • 1
  • 9
  • 16
  • 2
    Sidenote: [do not cast the result of `malloc`](https://stackoverflow.com/questions/605845/) – Marco Bonelli Jun 07 '20 at 02:00
  • Can you elaborate? What you are trying to do in this whole code? – Shubham Jun 07 '20 at 02:21
  • 1
    @Miguel Humberto This loop for(i=0;i<5;i++){ (*arr)[i] = (int*)malloc(gd*sizeof(int)); } invokes undefined behavior. – Vlad from Moscow Jun 07 '20 at 02:39
  • I'm trying make dynamic 2d array to safe int values, so, I use functions and to allocate and scan the values and show it – Miguel Humberto Jun 07 '20 at 02:42
  • @Vlad from Moscow that's weird. I found another guy tried to allocated a dynamic 2d array, and the best answer use that loop – Miguel Humberto Jun 07 '20 at 02:48
  • 1
    @MiguelHumberto I do not know where you have found such a "best answer" but as I said the program has undefined behavior and does not make sense. – Vlad from Moscow Jun 07 '20 at 02:50
  • 2
    @MiguelHumberto In this statement *arr = (int**)malloc(gd*sizeof(int*)); you allocated an array of two pointers. But in this loop for(i=0;i<5;i++){ (*arr)[i] = (int*)malloc(gd*sizeof(int)); you are trying to access 5 pointers in the array. – Vlad from Moscow Jun 07 '20 at 02:52
  • @Maeco Bonelli I cast malloc cause maybe I'm not using C99 (i dont what C9# is because my IDE dont show me it) – Miguel Humberto Jun 07 '20 at 02:53
  • 1
    Suggest `int **getmem(void) {...}` and then assign the return in `main()`, e.g. `int **arr = getmem();` Being a [Three Star Programmer](http://c2.com/cgi/wiki?ThreeStarProgrammer) is generally not a compliment. Don't forget to validate that EVERY allocation succeeds. – David C. Rankin Jun 07 '20 at 03:09
  • OT for robust code, regarding: `(*arr)[i] = (int*)malloc(gd*sizeof(int));` 1) do not cast the return value. such casting just clutters the code and is error prone. The returned type is `void*` which can be assigned to any pointer. Suggest removing that cast. 2) always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Jun 07 '20 at 18:13
  • regarding: `void get_mem(int ***arr);` please read: [3 star programmer](https://wiki.c2.com/?ThreeStarProgrammer) – user3629249 Jun 07 '20 at 18:17
  • OT: for ease of readability and understanding: 1) please consistently indent the code. Indent after every opening brace '{'. Unindent before every closing brace '}'. Suggest each indent level be 4 spaces. – user3629249 Jun 07 '20 at 18:18
  • OT: in function: `get_data()` regarding: `int c,f;` In general, it is best to limit the scope of variables as much as possible. Suggest removing that statement. Then writing the `for()` loops as: `for( int c=0; c – user3629249 Jun 07 '20 at 18:32
  • OT: In general, macro names, enum values, etc are written in ALL_CAPS with an underline character separating the root words. Therefore, it would be better if `#define gd 2` be written as: `#define GD 2` – user3629249 Jun 07 '20 at 18:34
  • OT: for ease of readability and understanding: (the compiler does not care) 1) please insert a (reasonable) space: after commas, after semicolons, inside parens, inside braces, inside brackets, around C operators 2) please separate code blocks: `for` `if` `else` `while` `do...while` `switch` `case` `default` via a single blank line. 3) separate functions by 2 or 3 blank lines (be consistent) – user3629249 Jun 07 '20 at 18:39
  • regarding: `fpurge(stdin);` andf `fflush(stdin); ` 1) the is no C function: `fpurge()` and the posted code does not contain such a function. 2) the function: `fflush()` is ONLY for output streams (visual studio has added such a function, but that makes visual studio non-compliant with the C language. Suggest using: `int ch; while( (ch = getchar() ) !=EOF && ch != '\n' ){;}` – user3629249 Jun 07 '20 at 18:48
  • OT: regarding: `scanf("%2d",&*(*arr+c)+f);` for robust code, it is best to check the returned value (not the parameter values) to assure the operation was successful. Note: the `scanf()` family of functions returns the number of successful 'input format conversion' specifiers (or EOF) – user3629249 Jun 07 '20 at 18:55
  • the posted code contains a number of memory leaks. Before calling: `free(*arr);` (which should be `free( arr );` need to pass to free the individual bits of allocated memory. Suggest: `for( int i = 0; i – user3629249 Jun 07 '20 at 18:59
  • regarding: `scanf("%2d",&*(*arr+c)+f);` that does not work as desired, suggest: `scanf( "%d", *arr+(c*GD)+f );` – user3629249 Jun 07 '20 at 19:14

2 Answers2

4

The value of macro gd is 2. In get_mem(), allocating memory for 2 int *:

    *arr =  (int**)malloc(gd*sizeof(int*));

and below it, accessing arr beyond it size:

for(i=0;i<5;i++){    //allocating memory to 5 int pointers
          ^^
    (*arr)[i] = (int*)malloc(gd*sizeof(int));
}

Accessing an unallocated memory is undefined behaviour.

Instead of using magic number 5 in the loop condition, you should check i with gd, like this

for(i=0;i<gd;i++){

In get_data(), the way you are accessing elements of arr for input is wrong

scanf("%2d",&*(*arr+c)+f);
            ^^^^^^^^^^^^ 

because

&arr[c][f] --> &(*(arr[c] + f) --> &(*(*(arr + c) + f)) --> &*(*(arr + c) + f) --> (*(arr + c) + f)

Note: The operator & is used to get the address and the operator * is used for dereferencing. These operators cancel the effect of each other when used one after another. Hence, &(*(arr + i)) is equivalent to arr + i.

That means, &arr[c][f] is equivalent to (*(arr + c) + f) and you should use &arr[c][f] which is less error prone and more readable:

for(f = 0; f < gd; f++) {
    scanf("%2d", &arr[c][f]);

Same mistake you have made while printing the arr elements in second for loop:

for(f=0;f<gd;f++){
    printf("%d ",*(*arr+c)+f);
                 ^^^^^^^^^^^

It should be *(*(arr + c) + f). More readable form is arr[c][f]:

for(f = 0; f < gd; f++){
    printf("%d ", arr[c][f]);

You should not use fflush() for input stream. It's undefined behavior. From C Standards#7.21.5.2p2 -

If stream points to an output stream or an update stream in which the most recent operation was not input, the fflush function causes any unwritten data for that stream to be delivered to the host environment to be written to the file; otherwise, the behavior is undefined.

Also, fpurge() is nonstandard and not portable. Moreover, you don't need to use either of them.

H.S.
  • 11,654
  • 2
  • 15
  • 32
  • The bigger point I see is there is no need to allocate pointers and then allocate blocks for `int` storage. If I understand, they are just reading the CSV and taking the first two chars of each to convert to `int`. All that is needed is allocation of a single block for the `int` values, not both pointers and `int`. (that would only be needed if they were storing the first two chars of each CSV as *strings* (or raw `char` or multiple rows of CSV)) . But it is not 100% clear. – David C. Rankin Jun 07 '20 at 03:45
  • @DavidC.Rankin Thanks, made correction. I agree with you regarding the allocation of single block of the `int` values and all but OP did not ask anything about the best/appropriate way of parsing such input. OP asked - _I limited the scanf len and my problem is when input a value over the limit, something weird happen_ which is occurring because of a mistake which is not easy to catch (apart from memory allocation problem). I hope OP will learn and add something to his knowledge from my answer. Indeed the appropriate approach is good to add but I think it's out of scope for this question. – H.S. Jun 07 '20 at 04:07
  • I liked the answer, and the biggest problem was the clarity of the question. Whether the input and output was for a MCVE with only a single line of input, or whether it was actually for a larger set. The 3-Star Programmer approach was a bit wonky as well, but you answer the direct question of "Unexpected outputs, why?". – David C. Rankin Jun 07 '20 at 04:11
  • @HS This code is like a test for myself, I tried to developed this like a grade list, or something similar, to save valors like those, i know can use single pointer and avoid all this, but is a challenge to me. I pretty newby in Programing world, and although this dont justify my stupid question and error. thanks for help – Miguel Humberto Jun 07 '20 at 05:59
  • Regarding fpurge (), I need to use it, I don't know if it is for not using c99, but if I don't put it, the buffer fills up and I can only read 2/4 values. – Miguel Humberto Jun 07 '20 at 06:26
2

Rather than using more & more pointers, I'd like to do it in this way:-

#include <stdio.h>
#include <stdlib.h>
//#define gd 2
#define ROW 2
#define COLUMN 2

void get_mem(int ***arr);
void get_data(int **arr);

int main(){
    int **arr = NULL;
    get_mem(&arr);
    printf("Enter 4 int values: ");
    get_data(arr);
    free(*arr);
    return 0;
}

void get_mem(int ***arr)
{
    int i;
    *arr = ( int ** )malloc( ROW * sizeof(int *) );

    for(i = 0; i < COLUMN; i++)
    {
        (*arr)[i] = ( int * )malloc( COLUMN * sizeof(int) );
    }
    printf("Okay!\n");
}

void get_data(int **arr)
{
    int c, f;
    for(c = 0; c < ROW; c++)
    {
        for(f = 0; f < COLUMN; f++)
        {
            scanf("%2d", &arr[c][f]);   //*(*arr+c)+f)
        }
    }
    for(c = 0; c < ROW; c++)
    {
        for(f = 0; f < COLUMN; f++)
        {
            printf("%d ", arr[c][f]);   //*(*arr+c)+f)
        }
        putchar('\n');
    }
}

I don't know what that gd is but it was making code ambiguous so I removed that and replaced it with ROW & COLUMN everywhere in the program(where it was necessary).

After allocating space to int **arr via get_mem() function, at least ask the user to input values and use proper spacing & indenting.

There's no need of fflush or fpurge, so I removed them. Now, here if you're accessing array this way you need to be very careful of using parenthesis at proper places. You should use *(*(arr+c)+f) instead of *(*arr+c)+f)(It was an error.) this. But I chose to access the elements or store values as we do in 2D arrays. That's easier.

If you want to access this array using pointers only, instead of arr[c][f] you can do it in this way:-

scanf("%2d", &(*(*(arr+c)+f)));

&

printf("%d ", *(*(arr+c)+f));

Note: Also, you should check for any error while allocating memory.

Hope, it helps.

Shubham
  • 1,153
  • 8
  • 20
  • 1
    I think the purpose of `gd` was to limit the number of characters converted from each comma separate value to `2`. Meaning if the string `1111` was read, then `gd` would limit the conversion to `11`. But I'm not entirely clear on the point. Your definition of `ROW` and `COLUMN` is good, I generally just use `ROWS` and `COLS` just for tidy consistent spacing and to convey the plural rather than the singular, but that is just a habit thing.... – David C. Rankin Jun 07 '20 at 03:56
  • 1
    Yeah, sometimes, when I did code myself (like challenges for develop new things), i use words like gd, or something else, my bad. But gs is like you say, colum and row. Is a 2d square array 2x2. Thanks for help – Miguel Humberto Jun 07 '20 at 06:02
  • Its your code @MiguelHumberto, do whatever you wanna do with it. But please do us a favor, make it more readable and understandable by the people of this community, before uploading any question. That would be very helpful to us to answer you. – Shubham Jun 07 '20 at 14:30