-6

I have something of this sort:

int n,m;
scanf("%d %d",&m,&n);
int *arr = malloc(sizeof(int)*n*m);
for(int i=0;i<m*n;scanf("%d",arr+i),i++);

Now for say n=2 and m = 3, It accepts the first five numbers, and gives a segmentation fault on the sixth number. I tried printing stuff immediately after the loop, but its not being printed, any idea what the problem could be? I've used similar constructs extensively and have never encountered a problem before.

EDIT 1: The problem was later on in the program, but the thing is I had a printf immediately after the loop, and it didn't print anything, so I assumed that it had to be here. Why didn't the printf print anything? Does it have something to do with parallel execution? And sorry for the bad format, I'm new to stack overflow.

5
  • 1
    Read about undefined behavior and buffer overflow. Compile with all warnings & debug info (gcc -Wall -Wextra -g). Use the debugger gdb and valgrind. Next time you ask some question on SO, give some MCVE. This fix my code question is off-topic. Read also documentation, notably of malloc and scanf Oct 1, 2017 at 7:08
  • 6
    "something of this sort".... Please study the concept of a minimal reproducible example.
    – Yunnosch
    Oct 1, 2017 at 7:10
  • Check in the debugger the values (with a breakpoint on the for loop) of m, n, arr. I'm sure you'll be surprised. Oct 1, 2017 at 7:16
  • 2
    This is by far the worst way of writing a loop I have seen. Oct 1, 2017 at 7:22
  • Please stop writing 'clever' code. It's confusing and difficult to debug. Oh - and it often goes wrong:( Developing software, which includes testing, debugging and maintenance, is difficult enough without over-complicating and almost-obfuscating the source code. Oct 1, 2017 at 10:45

2 Answers 2

1

Try this,

int n,m;
printf( "Enter two digits" );
int scanCount = scanf( "%d %d", &m, &n );
if( scanCount < 2 ){   
    perror("Input");
    exit(EXIT_FAILURE);   
}
scanCount = 0;
size_t size = sizeof( int ) * n * m;
int * arr = (int *) malloc( size );
if ( arr == NULL ) { 
    perror("malloc");
    exit(EXIT_FAILURE);
};
for( int i = 0; i < m * n; i++ ){
    if ( scanf( "%d", arr + i ) )
        scanCount++;
}
if( scanCount < m*n ){   
    perror("Input");
    exit(EXIT_FAILURE);
}
for( int i=0; i<n*m; i++ ){
    printf( "\nValue at %d : %d\n", i, *( arr + i ) );
}

I have modified the above given program a little bit, and now it works for me. The malloc function expects "std::size_t size" ( we can also provide integer values ) as argument which is the size of the memory we need to allocate. Please visit http://en.cppreference.com/w/cpp/memory/c/malloc for any reference.

Also we should specify the type of the memory we are creating ( otherwise it may produce an error in some compilers, like this "invalid conversion from 'void*' to 'int*'" ). In this case we are creting an array of integers and we can use type cating, for example (int *) malloc( size).

It is better to implement some error handlers for failed memory allocation and inputs through scanf ( check the return values of malloc and scanf )

10
  • 3
    Missing check of malloc failure: if (arr==NULL) { perror("malloc"); exit(EXIT_FAILURE);}; and useless use of cast for malloc; BTW scanf can also fail and your code don't check that. So please edit your answer to improve it. Oct 1, 2017 at 7:32
  • 1
    So edit again your answer to improve it. Then I will upvote it! Oct 1, 2017 at 8:13
  • 1
    Thanks for having improved your (probably first) answer on SO. I upvoted it and I believe it should be the accepted answer. You should improve it even more by adding relevant links to documentation and a few sentences of explanations. BTW, I believe you should remove the empty lines in your code Oct 1, 2017 at 8:53
  • 1
    @BasileStarynkevitch thank you so much, your comments were so informative. Glad to solve problems with the help of people like you :)
    – HariV
    Oct 1, 2017 at 8:58
  • 1
    You really should edit your answer a bit more to explain in English sentences something about it. A "code only" answer is poor taste on SO. Oct 1, 2017 at 9:02
1

I tried running your program here. No error. Perhaps that was coincidence and just happened to run but maybe you got error when you printed the values inside the loop like this?

If so, that's because during the last iteration you try to write a part of memory that wasn't allocated to using malloc(). Like arr[6] when only till arr[5] was actually allocated when n=2 and m=3. Doing so invokes undefined behavior. Look at the links @Basil pointed out in the comments.

You must check the value returned by malloc() to find if the memory allocation was successful. If it failed, a NULL would be returned.

And the scanf()'s return value may be checked to find if it was successful or not. It returns the number of successful assignments which in the case of the scanf() in the loop must be 1. If it isn't 1 some error has occurred.

This is another way of writing that loop

for( i=0;i<m*n && scanf("%d", arr+i)==1;i++);

Not the answer you're looking for? Browse other questions tagged or ask your own question.