-4

Here is my code:

int main () 

{
    const int MAX = 10;
    char *buffer = nullptr;       // used to get input to initialize numEntries
    int ARRAY_SIZE = 20;            // default is 20 entries

    int numEntries;
    success = false;

    while (!success)
    {
        delete buffer;
        buffer = nullptr;
        buffer =  new char [MAX];

        cout << "How many data entries:\t";
        cin.getline(buffer, MAX, '\n');

        cout << endl << endl;

        while (*buffer)
        {
            if (isdigit(*buffer++))
                success = true;
            else
        {       
               success = false;
                break;
        }
    }
}

numEntries = atoi(buffer);

The problem is that when I enter a any number, it just displays "numEntries = 0", and it crashes if I enter a string.

Can someone explain what exactly is happening?

Bbvarghe
  • 267
  • 2
  • 5
  • 18
  • 4
    Delete arrays with `delete []` – aggsol Jun 11 '13 at 05:57
  • 2
    You'd have less troubles using a `std::string` instead of a `char*` : http://www.cplusplus.com/reference/string/string/getline/ – Thomas Ruiz Jun 11 '13 at 05:58
  • 4
    Your code doesn't match the question: where do you print "numEntries = ??" ? Also your while loop is buggy `success` will be set according to the last char in the array, not the whole thing. – John3136 Jun 11 '13 at 05:59
  • 3
    This code is simply broken. You allocate an array of `char` and store it in `buffer`, but then you move `buffer` to point somewhere else. Calling `delete` on it later is simply undefined behaviour (and it should be `delete[]`, since you allocated an array). – Angew is no longer proud of SO Jun 11 '13 at 06:01
  • Also, you increment the `buffer` every time you check for `isdigit`. If all the characters are digits, you get out bound at the end and then you are trying to delete that value of `buffer` in the next iteration, not the actual base pointer of that array. – raj raj Jun 11 '13 at 06:12
  • 2
    @rajraj Calling `delete` (or `delete[]`) on a null pointer is a guaranteed no-op, no need to check beforehand. – Angew is no longer proud of SO Jun 11 '13 at 06:33
  • @Angew I'm very new at this so it would be great if you explained: - where does the buffer point somewhere else? - I was told to use delete when allocating memory. Should I be doing something else? – Bbvarghe Jun 11 '13 at 14:35
  • @Bbvarghe `*buffer++` increments the `buffer` pointer. So it points somewhere else. `new` must be paired with `delete`, `new[]` must be paired with `delete[]`. See also KazDragon's answer. – Angew is no longer proud of SO Jun 11 '13 at 14:53
  • @Angew *buffer++ is supposed to traverse the buffer array. should it be while(*(buffer++))? – Bbvarghe Jun 11 '13 at 15:16
  • @Bbvarghe It traverses the array *by moving the `buffer` pointer to successive elements.* This is pretty basic stuff - you should probably pick up a [good C++ book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – Angew is no longer proud of SO Jun 11 '13 at 15:19

1 Answers1

0

This is your problem:

+----+----+----+----+----+
|    |    |    |    |    |
+----+----+----+----+----+
  ^                         ^
  |                         |
 Start                     End
  • Start is the pointer returned from the new [] expression.
  • The second while loop while (*buffer) iteratively moves the pointer to the position denoted by End.
  • End is passed to delete. This is wrong. Start wants to be passed to delete.

What you may want to do is store a second pointer to the result of the new [] expression to be used with delete (which should also be delete []) For example:

int main () 
{
    const int MAX = 10;
    char *buffer = nullptr;       // used to get input to initialize numEntries
    char *bufferptr = nullptr;    // <- ADDED
    int ARRAY_SIZE = 20;            // default is 20 entries

    int index = 0;
    success = false;

    while (!success)
    {
        delete [] bufferptr; // <- CHANGED
        buffer = nullptr;
        buffer =  new char [MAX];
        bufferptr = buffer; // <- ADDED

        cout << "How many data entries:\t";
        cin.getline(buffer, MAX, '\n');

        cout << endl << endl;

        while (*buffer)
        {
            if (isdigit(*buffer++))
                success = true;
            else
                success = false;
        }
    }
}
Kaz Dragon
  • 6,681
  • 2
  • 36
  • 45
  • how does deleting bufferptr delete buffer? – Bbvarghe Jun 11 '13 at 14:58
  • @Bbvarghe Because you set `bufferptr` to `buffer` before moving `buffer` away. So `bufferptr` will point to the dynamic array which needs deleting. See also my comment on books, directly at the question. – Angew is no longer proud of SO Jun 11 '13 at 15:20
  • So, that means buffer = new char [MAX]' is not setting buffer to an array of pointers where each index points to a different character of the string, but rather it stores the pointer for an array of chars that holds the string – Bbvarghe Jun 12 '13 at 06:43
  • @Bbvarghe Precisely. And since you move that pointer during the second while loop, it cannot be used in the delete operation. My proposed change caches the original pointer returned by new[], which is then passed to delete[]. – Kaz Dragon Jun 12 '13 at 06:58