Problem with simple piece of code

I'm having a bit of trouble with an excercise for C++ (for reference, I use SSH to connect to my University and code using Unix with g++ compiler).

This simple piece of code (which after much stress over being unable to get the function to work on my own) I copied directly from a sample excercise and it still gives me the following when I try to excecute it: "Segmentation Fault". Can somebody tell me - what's wrong here?

Code:

#include <iostream>
#include <stdlib.h>
using namespace std ;

void main( int argc, char* argv[] )
{
 int  marks[5] ;
 int total = 0 ;

  for ( int i = 0; i < 6; i++)
    {
      marks[i] = atoi(argv[i+1]) ;
    }
}

P.S. Can somebody also explain what a segmentation error is in the first place?

To start from the back:
segmentation fault means that you are accessing memory that you do not own - for example trying to access memory through a NULL pointer, or a "random value" pointer.

Code:

  int  marks[5] ;
  ... 
  for ( int i = 0; i < 6; i++)
    {
      marks[i] = atoi(argv[i+1]) ;
    }

You have an array of 5 elements. You use 6 elements of it: 0..5 will be the values of i.

That is unlikely to be the cause of segfault. My guess would be that you do not have 6 arguments for your program itself. What does your command-line for the program run look like?

--
Mats

Code:

#include <iostream>
#include <stdlib.h>
using namespace std ;

void main( int argc, char* argv[] )
{
 int  marks[5] ;
 int total = 0 ;

  for ( int i = 0; i < 5 && i < argc; i++)
    {
      marks[i] = atoi(argv[i+1]) ;
    }
}

Ah, I did not know this error would appear if I did not pass enough arguments. I was at first using this code (below), but I was having a problem with writing <stdlib.h> as <stdlib> and in checking what was wrong I tried replacing "argc - 1" as size of arrays with actual numbers to see if that was a problem. After fixing the real problem I ended up still having problem because I never tried reverting back to the original code.

Oh, and in the code edited by c++0x what exactly is "argci"?

This (below) should work, correct?

Code:

#include <iostream>
#include <stdlib.h>
using namespace std ;

int main( int argc, char* argv[] )
{
 int  marks[argc - 1] ;
 int total = 0 ;

  for ( int i = 0; i < argc - 1; i++)
    {
      marks[i] = atoi(argv[i+1]) ;
    }
}

Segmentation fault is a signal that your linux operating system sends to your program when it detects your program accessing or overwriting an area of memory that it shouldn't. The signal causes your program to abort and (optionally) report the error message you're seeing.

There are two potential contributors to this in your code;

1) marks is an array of length 5 (with values marks[0] through to marks[4]). Your code is modifying marks[5] which does not exist (due to the i < 6 end condition for the loop).

2) Your loop is evaluating atoi(argv[i+1]) (for i = 0 to 5, inclusive) without checking the value of argc. If you have not run your code with 6 command line arguments, these argv[i]'s will not exist.

Formally, because of these things, your code is said to exhibit undefined behaviour (the C++ standard says nothing about segmentation faults). According to the standard, anything is allowed to happen with code that exhibits undefined behaviour: no compiler diagnostics are required, any symptom can occur.

Unrelated to your problem, but something to be aware of;

1) main() returns int, not void. Any book that gives examples of main() returning void is implicitly of low quality and reliability.

2) instead of <stdlib.h>, it is better to use <cstdlib> in C++. <stdlib.h> is not disallowed, but is discouraged (deprecated) by the C++ standard. The main practical difference between <cstdlib> and <stdlib.h> is that functions/symbols are placed within namespace std: your code has a "using namespace std;" directive, so will be - mostly - unaffected by that difference.

Oh, and in the code edited by c++0x what exactly is "argci"?

A typo.

This (below) should work, correct?

No, this isn't C++. The size of an array must be known at compile time, and argc-1 is a runtime expression.


You can either use a statically sized array and ensure that you don't exceed its bounds, or you can use a dynamic array. The easiest form of dynamic array would be a std::vector.

Sorry, by "work" I meant the code will do what I want it to (I generally try to ensure I can hand in the excercise and get the marks before I go about seeing how it can be done better). However, thanks for the suggestion, I will look up dynamic arrays for future.

Though I have to say, the quality and speed of receiving answers is excellent so thank you all for spending your time to help people like me.

I fuxed the other code with the currection by CormedBee.

Code:

#include <iostream>
#include <stdlib.h>
using namespace std ;

int main( int argc, char* argv[] )
{
 int  marks[--argc];
 int total = 0 ;

  for ( int i = 0; i < argc; i++)
  {
    marks[i] = atoi(argv[i+1]) ;
  }
}

U may wish 2 do someting like this

Code:

#include <iostream>
#include <new>

int main( int argc, char** argv )
{
 try
 {
  int  *marks = new int[--argc];
  int total = 0 ;

  for ( int i = 0; i < argc; i++)
  {
   marks[i] = atoi(*(++argv)) ;
  }

  delete[] marks;
  return 0;
 } catch(std::bad_alloc e)
 {
  return -1;
 }
}

Still wrong. If you disable compiler-specific extensions, neither

int marks[argc-1];

nor

int marks[argc--];

will compile. And the latter is far too tricky.

Well....

Code:

#include <iostream>
#include <new>

int main( int argc, char** argv )
{
 try
 {
  int  *marks = new int[--argc];
  int total = 0 ;

  for ( int i = 0, j = 1; i < argc; i++)
  {
   marks[i] = atoi(argv[j]) ;
  }

  delete[] marks;
  return 0;
 } catch(std::bad_alloc e)
 {
  return -1;
 }
}

And new[--argc] is just a syntax error no matter how you slice it.