-2

I am currently writing a script about finding the nearest duplicate from a user entered size array. The array must be between 1 and 10^5 and its value has to be between 1 and 10^5 also. It compiles normally on my computer but whenever I submit it, it returns a run_error.

Here's what I wrote.

#include<stdio.h>
#include<math.h>
#include<stdlib.h>
void input (int *);
int main(){
    int n;
    input (&n);
    int a[n];

    for(int i=0;i<n;i++){
        if (scanf("%d",&a[i])!=1) 
            return 0;
        if ((a[i]>100000)||(a[i]<1))
            return 0;

        }

    for (int i=0;i<n;i++){
        if (a[abs(a[i])]>=0){
            a[abs(a[i])]=-a[abs(a[i])];
        } else {
            printf("%d",abs(a[i]));
            return 0;
        }
    } 
    return 0; 
}

void input(int *x){
    if (scanf("%d",x)!=1)
        exit(0);
    if ((*x>100000)||(*x<1))
        exit(0);
}
Baran
  • 97
  • 1
  • 11
  • `int a[n];` potentially allocates a very large amount of memory on the stack. Make it a global or use malloc. – Retired Ninja Jun 09 '19 at 06:14
  • `a[abs(a[i])]` this will be out of bounds when `a[i] = 100000` – Avin Kavish Jun 09 '19 at 06:15
  • @RetiredNinja I wouldn't call 100,000 a large amount of memory. Stack should be fine. – Avin Kavish Jun 09 '19 at 06:16
  • 1
    @AvinKavish Not all stacks are created equal. I'd play it safe. – Retired Ninja Jun 09 '19 at 06:20
  • Variable length arrays are not part of standard C++. Some compilers support VLAs as an extension, but you really shouldn't use them. Use a `std::vector` when you need a dynamic array and `std::array` when you need a fixed size one. – Jesper Juhl Jun 09 '19 at 06:40
  • The usage of `int a[n]` is not even valid C++. Unfortunately, it is a non-standard extension that some C++ compilers support. If it is supported, the array sizes possible are significantly smaller than is possible using `malloc()`, since it uses "stack" memory - most modern operating systems limit processes to a small quota of stack. – Peter Jun 09 '19 at 10:10

2 Answers2

0

This program is logically incorrect. There is no connection between the size of the array defined in n and the limit of the allowed elements.

Since you have allowed a[i]>100000 to go up to 10^5 with no regards to the size of the array defined by a[n], the following access will attempt to access outside the bounds of the array, a[abs(a[i])] for any a[i] > n.

Also, you can pass by reference for syntactical simplicity

input(n);

void input(int &x){
    if (scanf("%d",&x)!=1)
        exit(0);
    if (x>100000 ||x<1)
        exit(0);
}
Avin Kavish
  • 8,317
  • 1
  • 21
  • 36
0

First of all, if you are writing in C, please tag this question as C question, and not as C++ one. scanf & printf are C functions, as much as your includes stdio.h & stdlib.h & math.h. In c++ you have the include iostream, and in this case it's all you really need.

The second problem here is the way you handle the input validation when it wrong. exit is very dangerous way, and very not recommended. If you want to throw exception use throw method (Read about the difference between the two: https://stackoverflow.com/a/56406586/8038186). But, in this case I don't understand why do you need to throw exception at all. you can simply finish the program in more gentle way. Consider the following flow:

#include <iostream>

using namespace std;

// Num is moved by reference and not by pointer
bool input(int &num) { // return true if the input is good, otherwise return false.
    cout << "Please enter number: " << endl; // Tell the user that he should enter a number.
    cin << num;
    bool is_valid = num >= 1 && num <= 1e5; // 1e5 = 100000
    if (!is_valid) cout << "Invalid input." << endl;
    return is_valid;
}

int main() {
    int n;
    if (!input(n)) {
        return 0;
    }
    int a[n];
    int i;

    for (i = 0; i < n && input(a[i]); i++);
    if (i < n) {
        return 0;
    }

    for (i = 0; i < n; i++) {
        // The following if is useless, the validation make sure already that all of the numbers are legal.
        //if (a[a[i]] >= 0) { // You don't need abs(a[i]), a[i] > 1
        if (a[i] < n)
            a[a[i]] = -a[a[i]];
        else cout << "Consider what do you want to do" << endl;
        /*} else {
            printf("%d", a[i]);
            return 0; 
        }*/
    } 
    return 0; 
}

Read about the difference between reference and pointer: What are the differences between a pointer variable and a reference variable in C++?

Coral Kashri
  • 3,436
  • 2
  • 10
  • 22