Re: Function with two int parameters

From: George Huber (kharmon_at_optonline.net)
Date: 03/12/05 

Date: Sat, 12 Mar 2005 09:59:48 -0500

On Sat, 12 Mar 2005 04:21:53 +0000, guest wrote:

[...snip...]

I `clean-up' the spacing of your code to make it easier for me to read,

>
> class Who
> {
> public:
> Who()
> {
> itsCards[20] = new int;
> itsPoints = new int;
> };
>
> ~Who()
> {
> delete [] itsCards;
> delete itsPoints;
> };
> int getCards(int cardnumber) const {return *itsCards[cardnumber];}
> void setCards(int cardnumber, int valuetoset)
> {
> *itsCards[cardnumber]= valuetoset;
> }
> getPoints() const {return *itsPoints;}
> void setPoints(int numberofpoints) {*itsPoints = numberofpoints;}
>
> private:
> int *itsCards[20];
> int *itsPoints;
> };
>

Now lets ignore main for a while and look at the implementation of the
class. Lets start with the first private member 'int* itsCards[20]', you
should be asking yourself what this line is actually doing.....

You are declaring that itsCards will be a twenty element array of pointers
to integers. So in memory you have something like this:

  +--------+--------+--------+ ....... +--------+
  | | | | | |
  | junk | junk | junk | | junk |
  | | | | | |
  +--------+--------+--------+ ....... +--------+

Notice that I have placed `junk' in each of the boxes - this is because at
this point the compiler has reserved space for twenty pointer, but not the
space for what they will point to.

Now for the constructor,
> Who()
> {
> itsCards[20] = new int;
> itsPoints = new int;
> };
In the first line we are attempting to access on of the element of the
array (which run from 0 to 19). First error: you are over-running the
bounds of the array. Even if the array index was in bounds, say 15, all
you would be doing in initializing that element of the array.

Now lets consider what happens when the member function setCards.
Set cards takes two args, the first is the index into the array of
pointers, and the second is the value to set. So lets step-through
the call `setCards(0, 12)'.

We first are going to get the elements at location zero of the array
(which is currently 'junk' because the array elements have not been
initialized). Once we have that pointer we are going to attempt to
dereference it - but the pointer value is 'junk' (in all probability
it is zero) which causes the segmentation fault.

I would make the following changes to your code:

1. Use a constant to represent the size of the array, makes life
   easier if you want to change the size later on, i.e.
 
   #define MAX_CARDS 20

2. Change the constructor to read:
    Who()
    {
        for(int idx = 0, idx < MAX_CARDS; idx++)
        {
            itsCards[idx] = new int;
            setCard(idx, 0);
        }
    }

3. Change the destructor to read:
   ~Who()
   {
       for(int idx=0; idx < MAX_CARDS; idx++)
       {
           if(NULL != itsCards[idx])
           {
               delete itsCards[idx];
               itsCards[idx] = NULL;
           }
       }
   }
   Note, some people might take `exception' to my explicitly setting the
   pointer to `NULL'. They would say that since the object is being
   deleted there is no way to access the pointer again, so there is no
   reason to set it to NULL (which is true). But the memory can be
   reused, so it is possible that another instance of class Who
   created after this one is destroyed would get the same memory, and
   I want to insure (as much as I can) that I know the state of the
   machine.

4. Change the setCard function to read:
   setCard(card idx, value val)
   {
       if(NULL != itsCards[idx])
           *itsCards[idx] = val;
   }

5. Insure anyother access into the array itsCards verifies that the
   value of the array element is non-null prior to using it.

6. I would ask myself why I am using only pointers. I do not see that
   pointers gains you anything in this code, but they do open the door
   for all sort of subtle problems.

Relevant Pages

  • Re: Problem with large arrays
    ... >am trying to using an array of signals that is just slightly larger) for the ... location of this very large memory. ... so that each pointer points to one row. ... row data structure and make the pointer point to it. ...
    (comp.lang.vhdl)
  • Re: Out-of-bounds nonsense
    ... 6.5.6p8 of the C standard says about C pointer arithmetic. ... moving throught that array. ... The wording used in both standards makes sense only if the relevent ... pointer arithmetic within 'malloc'ed memory blocks (which naturally have no ...
    (comp.std.c)
  • Re: Problem with large arrays
    ... >>am trying to using an array of signals that is just slightly larger) for the ... > location of this very large memory. ... instead of signals I used variables and the snapshot was not any ... > so that each pointer points to one row. ...
    (comp.lang.vhdl)
  • Re: Creating and Accessing Very Big Arrays in C
    ... As for memory differences between storing multidimensional arrays ... memory a pointer takes up. ... if I wanted to store a 100x100x100 3D array ... If anyone knows of any good sparse matrix ...
    (comp.lang.c)
  • Re: char **argv & char *argv[]
    ... "pointer to pointer to char". ... >> pointer)) pointing to the first element of an array. ... so we have to start adding more context. ... type "pointer to char", rather than "array MISSING_SIZE of char". ...
    (comp.lang.c)