Programming Puzzle - Find As Many Errors As You Can

[Hooman]

Ahh, I came across this fun little problem.


There are multiple things wrong with this code. Name as many of them as you can.

Code: cpp [Select]

#include <vector.h>

void main(int argc, char** argv)
{
  int n;
  if (argc > 1)
    n = argv[0];
  int* stuff = new int[n];
  vector<int> v(100000);
  delete stuff;
  return 0;
}

[leeor_net]

Code: [Select]

#include <vector.h> // Should be <vector>


void main(int argc, char** argv) // evil main declaration, anybody who does this deserves horrible death
{
  int n; // Uninitialized variable

  if (argc > 1) // poor formatting... nitpicky I know but also kind of a useless if expression as argc will _always_ be 1 or more (at least on windows)
    n = argv[0]; // implicit type cast from char* to int, will yield unexpected results especially for novices. Most compilers won't even let this build without complaining about it. Worse still, what's the intent here? This is a nonsensical statement.

  int* stuff = new int[n]; // Potential for blowing up due to possible uninitialized variable above.
  vector<int> v(100000); // namespace undeclared... requires std::. Also useless as not used anywhere.
  delete stuff; // using incorrect delete, need to use scalar delete []
  return 0; // main declared with void return type
}

Probably more stuff I haven't considered... but JFC, Hooman... how were you able to pack so much terrible into a tiny amount of code?

[Hooman]

Very well done Leeor! I'm impressed you caught some of that stuff. Did you do the clever thing and have a compiler help you out at all?    (That's what I always used to do).

As for the terrible code, it's from a list of C++ Interview Questions. Spoiler alert, it contains answers in that link.

A couple of additional points:

The argv[0] element is the program name, rather than the first argument. There is almost no reason to try and interpret it as an integer. Based on the check, and the implied integer conversion intent, it should probably be argv[1].

If the allocation for the vector throws, the memory allocated with new will be leaked.

[leeor_net]

Did you do the clever thing and have a compiler help you out at all?

In this case no, I just observed it. At least not until after I'd looked at it and responded. I did run it through a web compiler after the fact to see if there was anything I missed but... not this time. (I've learned a lot over the last few years)

The argv[0] element is the program name, rather than the first argument. There is almost no reason to try and interpret it as an integer. Based on the check, and the implied integer conversion intent, it should probably be argv[1].

Which is why I noted that it's a useless check. Granted it means that 'n' will always be initialized but what that value will be is undefined. If the compiler allows it at all it'll be a memory address and almost certainly not what the programmer is intending (which is probably a numeric value but that's an assumption and based only on the code, there is really no indication of intent).

If the allocation for the vector throws, the memory allocated with new will be leaked.

I'd considered this but (originally was going to mention no try/catch block for possible exceptions] but for int's unless you're on extremely limited hardware the likelihood of std::vector throwing in this case is low. Still though... robust programming and such.