0

I wrote a C program in Clion, using dynamic memory allocation. I noticed sometimes gave different output for the same input. At first I tought there was a problem with Clion (I was using it for the first time), but I also ran the program from the windows command line, and the same bug occured.


main.c

#include "main.h"

int main()
{
    int n;

    scanf("%d", &n);

    int *t = (int*)calloc(n, sizeof(int));

    for (int i = 0; i < n; ++i) {
        scanf("%d", &t[i]);
    }

    int p = peak(t, n);

    printf( p == -1 ? "No peak" : "Peak %d, position %d", t[p], p );

    free(t);

    return 0;
}

main.h

//
// Created by Botond on 2021. 02. 18..
//

#ifndef EXTRA_MAIN_H
#define EXTRA_MAIN_H

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

int peak( int *t, int n)
{
    int p = -1, i = 0;

    while(t[i] <= t[i+1])
        i++;

    p = i;

    while(t[i] >= t[i+1])
        i++;

    if(i == n + 1)
        return p;
    else
        return -1;
}

#endif //EXTRA_MAIN_H

different output for the same input from command line

Mike
  • 29
  • 1
  • 8
  • 6
    Your `peak` function has no checks to avoid running off the end of the array. – Nate Eldredge Feb 19 '21 at 22:22
  • 4
    Also you should generally not define functions in `.h` files. The `.h` file should only contain the declaration `int peak(int *, int);` and the code itself should go in a separate `.c` file which also includes the `.h` file, and which you compile separately and link together. – Nate Eldredge Feb 19 '21 at 22:25
  • I love this website – Mike Feb 19 '21 at 22:30
  • 1
    Regarding .c and .h files, https://stackoverflow.com/questions/5904530/how-do-header-and-source-files-in-c-work/5904752#5904752 might be good reading. – Nate Eldredge Feb 19 '21 at 22:30
  • 1
    Accessing outside the array bounds causes undefined behavior. That's why you can get different results. – Barmar Feb 19 '21 at 22:34
  • 1
    see also: [c - Do I cast the result of malloc? - Stack Overflow](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – MikeCAT Feb 19 '21 at 22:48
  • 1
    OT: regarding: `int *t = (int*)calloc(n, sizeof(int));` 1) in C, the returned type is `void*` which can be assigned to any pointer. Casting just clutters the code and is error prone. 2) always check (!=NULL) the returned value to assure the operation was successful. 3) the function expects its' parameters to be of type `size_t`, not `int` – user3629249 Feb 20 '21 at 08:13

1 Answers1

2

When the array passed to peak is {5,5,4,3,2,1}:

    while(t[i] <= t[i+1])
        i++;

will continue until i == 1. Then

    while(t[i] >= t[i+1])
        i++;

will still be true when i == 4, so the loop will iterate again and the test becomes t[5] >= t[6]. Since the array only has 6 elements (t[0]..t[5]), the access to t[6] causes undefined behavior. In particular, it could read whatever garbage value happens to be next in memory, so that your program behaves as if the input contained additional garbage values that might be different from one run to the next. The program could also crash or cause other sorts of problems.

You need an extra condition in these loops that checks i against n, so that they don't access t[n] and instead terminate when i+1 >= n.

Nate Eldredge
  • 48,811
  • 6
  • 54
  • 82