1

I'm trying to create a self-expanding array of a structure. I saw the questions here, here and here but the answers didn't seem to apply to my situation.

I have had pretty good luck using this technique using an array of strings, but using a structure doesn't work. Below is the code:

//  SO1.h
//
#pragma once

typedef struct {
    int tag;
    int type;
}structure;

void addElement(structure* Tkn_A, const int Tag);

void listArray();

Here is the C code.

// SO1.cpp : This file contains the 'main' function. Program execution begins and ends there.
//

#include "stdio.h"
#include "malloc.h"
#include "SO1.h"

const int   ARRAY_INITIAL_SIZE = 2;
const int   ARRAY_ADDITIONAL_SIZE = ARRAY_INITIAL_SIZE / 2;
structure* userArray;
size_t userArrayLength = -1;
size_t userArrayAvailable = ARRAY_INITIAL_SIZE;

int main()
{
    userArray = (structure*)malloc(userArrayAvailable * sizeof(structure));
    printf(" orgarrptr=%p\n", userArray);

    addElement(userArray, 13);
    addElement(userArray, 14);
    addElement(userArray, 15);
    addElement(userArray, 16);
    addElement(userArray, 17);
    addElement(userArray, 18);
    addElement(userArray, 19);
    addElement(userArray, 20);
    addElement(userArray, 21);
    addElement(userArray, 22);
    addElement(userArray, 23);
    addElement(userArray, 24);
    addElement(userArray, 25);
}

void addElement(structure* userArray, const int tag)
{
    userArrayLength++;
    if (userArrayLength > userArrayAvailable) {
        userArrayAvailable += ARRAY_ADDITIONAL_SIZE;
        structure* originalUserArrayPtr = userArray;
        printf(" orgarrptr=%p\n", originalUserArrayPtr);
        userArray = (structure*)realloc(userArray, userArrayAvailable * sizeof(structure));
        printf(" newarrptr=%p\n", userArray);
        if (originalUserArrayPtr != userArray) {
            printf("pointers different\n");
        }
    }
    userArray[userArrayLength].tag = tag;
    userArray[userArrayLength].type = 1;

    printf("%2d   %d\n\n", userArray[userArrayLength].tag,
        userArray[userArrayLength].type);

    listArray();
}

void listArray()
{
    for (size_t i = 0; i <= userArrayLength; i++) {
        printf("%2d   %d\n", userArray[i].tag,
            userArray[i].type);
    }
}

When the program doesn't complete normally, I get the following error on the line doing the realloc:

Debug Assertion Failed!

File: minkernel\crts\ucrt\src\appcrt\heap.cpp
Line: 604

Expression _CrtIsValidHeapPointer(block)

This happens when the realloc comes up with a different pointer than the original with a new length. But the next time I look at the pointer, it still has the value of the old pointer. Below is the output of one run:

:
:
:
13   1
14   1
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
20   1
 orgarrptr=015C79E0
 newarrptr=015C79E0
21   1

13   1
14   1
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
21   1
 orgarrptr=015C79E0
 newarrptr=015C5F58        <===  new pointer
pointers different
22   1

13   1
14   1
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
-842150451   -842150451
21   1
-1863261150   134281082
 orgarrptr=015C79E0        <===  original pointer


exited with code 3.

Does anyone know what would cause this problem? TIA.

Sorry for the long question, but I wanted to give out all the details. I don't think Visual Studio has anything to do with problems, but don't know for sure. I'm using VS Community Edition 2019, 16.1.3.

Update: Originally I had two problems. One getting the pointer updated and the other getting garbage data. I decided to split the two into two different questions.

Anonymoose
  • 38
  • 9
  • 1
    `void addElement(structure* userArray, const int tag)` works inside on the local variable `userArray` but no on the global variable `userArray` as you might expect. (The latter is eclipsed by the former inside the function.) Hence, whenever `realloc()` returns a new address, it gets lost as soon as you leave the function while the global `userArray` is kept unchanged. You could eliminate parameter `userArray` from `addElement()`. However, I would recommend to `return userArray` (changing the return type to `structure*`) and assign result of `addElement()` to the global again. – Scheff's Cat Jun 30 '19 at 07:55
  • 1
    @scheff. Why do you answer in a comment? – alk Jun 30 '19 at 08:18
  • 1
    @alk I'm just about to write an answer... – Scheff's Cat Jun 30 '19 at 08:19
  • 2
    In addition to the excellent answer of @Scheff, I think that, considering the capability of the function `addElement` to manage different allocations of the structure `structure`, it's better to manage the element counter (`userArrayLength`) in an other way. In the actual way you may manage only one array because the counter is managed using only one variable (`userArrayLength`). A simple way should be to pass the counter pointer to the `addElement` and the to the `listElement` functions. – Sir Jo Black Jun 30 '19 at 08:50
  • @AnttiHaapala I hadn't expected such duplicate as the issue has actually less to do with `realloc()` but more with scopes and eclipsing. Respect. ;-) – Scheff's Cat Jun 30 '19 at 09:08
  • @SirJoBlack: Thank you for your replies. I will update the code in a future version like you said. – Anonymoose Jun 30 '19 at 10:34

1 Answers1

3

IMHO, the OP is not yet aware about scopes of variables:

void addElement(structure* userArray, const int tag)
{
  /* The global variable userArray is now invisible (eclipsed).
   * Instead, the local parameter userArray is used.
   * It has its own storage.
   */
}

Hence, whenever realloc() returns a different address then given it gets lost after leaving addElement().

realloc() may return

  1. the given address (of first argument)
  2. a new address
  3. a NULL pointer.

The heap managment which is used inside of realloc() might recognize that memory after block to grow is still free. Hence, the block can be grown in place. It might also be the case that the heap management provided earlier a larger block than actually requested. (This could be a strategy to prevent fragmentation of heap memory.) This is the reason why same address might be returned although more memory than before was requested.

If nothing of above was applicable a new block is allocated, old contents copied to new address, and old block is freed. (Otherwise, it would become a memory leak.)

If realloc() fails to do any of the above, it returns NULL.

Recalling the above, it's even worse: Whenever realloc() returns a different address then the memory at old address is freed. So, the global userArray (which is not updated) becomes dangling (pointing to freed memory). Accessing a dangling pointer is Undefined Behavior → good for any strange effects including a crash (Access Violation).

Concerning the third:

userArray = (structure*)realloc(userArray, userArrayAvailable * sizeof(structure));

might appear questionable.

(My personal thinking about this is: When memory is exceeded this means usually things have gone very wrong. The process has usually to be terminated as soon as possible. This will happen on most platforms when a NULL pointer is accessed ignoring the fact that the ugly system error message might scare the user and force her/him to make a support call. And, please, note my repeated usage of "usually"...)

The OP already observed the points 1. and 2. in diagnostic output.

There are various options to fix this issue:

  1. get rid of the global variable eclipsing:
void addElement(const int tag)
{
  /* unmodified code will now access the global userArray */
}
  1. use a pointer to pointer instead:
void addElement(structure **userArray, const int tag)
{
  /* all uses of userArray in code have to get an additional indirection operator
   * i.e. replace all occurrences of "userArray" by "(*userArray)"
   */
}
  1. return (potentially changed) userArray as result:
structure* addElement(structure *userArray, const int tag)
{
  /* unmodified code */
  return userArray;
}

has to be called now:

userArray = addElement(userArray, 13);

to reassign the (potentially changed) pointer to global variable.


I (personally) prefer design 3 in general. However, considering that addElement() accesses a variety of other global variables as well option 1 seems to me the most consistent fix.

Scheff's Cat
  • 19,528
  • 6
  • 28
  • 56
  • 2
    In addition to your excellent answer, I think that, considering the function `addElement` capability (if the method 2 or 3 will used) to manage different allocations of the structure (`structure`), it's better to manage the element counter (`userArrayLength`) in an other way. In the actual way the code may manage only one array because the counter is managed using only one variable (`userArrayLength`). A simple way would be to pass the counter pointer to the `addElement` and the counter to the `listElement` functions. – Sir Jo Black Jun 30 '19 at 09:08
  • 1
    @SirJoBlack I fully agree. I'm afraid the answer is even long as it is. (I'm not yet good in the art of short answers.) So, I left an upvote on your comment... – Scheff's Cat Jun 30 '19 at 09:10
  • @Scheff: Thank you so much for the long and considerate answer. I went with option 3 and the problem is resolved. Thanks again. – Anonymoose Jun 30 '19 at 10:33