Thread: Problem with simple piece of code

  1. #1
    Registered User
    Join Date
    Dec 2008
    Posts
    66

    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?

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Banned
    Join Date
    Dec 2008
    Location
    Maputo, Mozambique
    Posts
    82
    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]) ;
        }
    }
    Last edited by c++0x; 12-12-2008 at 05:01 PM. Reason: Oops. Thanks CornedBeef. Good eye.

  4. #4
    Registered User
    Join Date
    Dec 2008
    Posts
    66
    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]) ;
        }
    }

  5. #5
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    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.

  6. #6
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    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.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  7. #7
    Registered User
    Join Date
    Dec 2008
    Posts
    66
    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.

  8. #8
    Banned
    Join Date
    Dec 2008
    Location
    Maputo, Mozambique
    Posts
    82
    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;
     }
    }
    Last edited by c++0x; 12-12-2008 at 05:27 PM. Reason: Tabstop u r my hero.

  9. #9
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    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.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  10. #10
    Banned
    Join Date
    Dec 2008
    Location
    Maputo, Mozambique
    Posts
    82
    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;
     }
    }
    Last edited by c++0x; 12-12-2008 at 05:26 PM. Reason: Thanks tabshop. Good eye.

  11. #11
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    And new[--argc] is just a syntax error no matter how you slice it.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Simple Code Problem
    By relyt_123 in forum C++ Programming
    Replies: 4
    Last Post: 07-10-2006, 11:19 AM
  2. problem with some simple code =/
    By Josh Norton in forum C++ Programming
    Replies: 3
    Last Post: 02-23-2004, 06:27 AM
  3. Request for comments
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 15
    Last Post: 01-02-2004, 10:33 AM
  4. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM
  5. Simple Compile Time Problem - HELP!
    By kamikazeecows in forum Windows Programming
    Replies: 2
    Last Post: 12-02-2001, 01:30 PM