1

I'm writing a C program. I want to read data from test.txt file. In my (.txt) file is a list of hexadecimal numbers 8-bit such as: 2A 8E 5A. Then I write these numbers to an array A. I have an error when I using Release mode in Visual Studio. Please tell me what's wrong?

int main(int argc, char *argv[]) {
    FILE *FileIn;
    int i, temp;
    int *A, nA = 0;
    A = (int *)realloc(NULL, sizeof(int));
    FileIn = fopen("test.txt", "r");
    // Add frist value to array.
    fscanf(FileIn, "%x", &temp);
    A[0] = temp;
    nA++;

    for(i = 0; i < 16; ++i){
        fscanf(FileIn, "%x", &temp);
        addValue(A, &nA, temp);
    }

    for (i = 0; i < 16; ++i) {
        printf("A[%d] = %x\n", i, A[i]);
    }
    free(A);
    system("pause");
    return 0;
}

void addValue(int *A, int *n, int Data)
{
     A = (int *)realloc(A, (*n+1)*sizeof(int *));
     A[*n] = Data;
     (*n)++;
}

enter image description here

Dang_Ho
  • 323
  • 3
  • 11
  • 1
    Have you debugged it line by line and checked that nA grows properly, allocations succeed and you never go past the end? – Sami Kuhmonen May 09 '17 at 09:14
  • 3
    `void addValue(int *A, int *n, int Data)` reallocs *A but assigns the result to a local pointer. The caller will never see the new value. – joop May 09 '17 at 09:15
  • 1
    realloc returns a value. Think why it needs to. Your addValue function doesn't return anything. It is just realloc with some irrelevant (for this discussion) stuff on top. Now if realloc returns a value and your function doesn't, this could mean one of the exactly two things. Either return value of realloc is redundant and you don't need it, or your function is missing something. Guess what is more likely. – n. m. could be an AI May 09 '17 at 09:20

3 Answers3

3

Here's what is really going on.

Let's take this example of some memory allocations packed together:

| some memory (10 bytes) | A (5 bytes) | unallocated (10 bytes) |

When you call realloc(A, 15) in the above scenario the system is able to expand the allocated memory at the same location:

| some memory (10 bytes) |             A (15 bytes)             |

However, what happens in the following case?

| some memory (10 bytes) | A (5 bytes) | some memory (10 bytes) |

This time, realloc(A, 15) can't simply expand the previously allocated region of memory since the latter 10 bytes it needs to expand A are currently allocated.

Instead, it has to do the equivalent of malloc()-ing a new memory region of 15 bytes and copying the original 5 bytes from A's old memory region to the first 5 bytes of the new region.

In that case, realloc() has not just expanded your memory allocation, but it has also moved the memory to a location where it can contiguously exist.

There are a few other reasons why realloc() would move memory (not just size requirements) but a lot of those are implementation defined.

Due to the possibility of a memory move, realloc() returns the address of the new pointer. As demonstrated above, sometimes it's the same pointer, and sometimes it's a new pointer.

When a move occurs, the old pointer is no longer valid as it is free()'d by the system. If you try to access it, you'll receive an access violation/segmentation fault.


In your example, you're realloc()-ing the memory in addValue() but you're never updating the caller's pointer. When the caller goes to use the old pointer, if a move occurred in addValue(), then the memory it tries to use will thus be invalid and the whole thing will crash and burn.

One way you could solve this is to pass a pointer to a pointer.

void addValue(int **A, int *n, int Data)
{
     *A = realloc(*A, (*n+1)*sizeof(int *));
     (*A)[*n] = Data;
     (*n)++;
}

And call it appropriately:

addValue(&A, &nA, temp);

A final note: In C, you don't need to cast the result of malloc() or realloc() - unlike C++, void* is implicitly cast-able to any other pointer type.

Qix - MONICA WAS MISTREATED
  • 14,451
  • 16
  • 82
  • 145
  • "When a move occurs, the old pointer is no longer valid as it is free()'d by the system. If you try to access it, you'll receive an access violation/segmentation fault." -- actually that would be the optimistic prediction. In reality, what will happen is you'll scribble all over malloc's data structures and corrupt allocated and unallocated memory in subtle and mysterious ways. – JeremyP May 09 '17 at 09:46
  • @JeremyP is a `free()` upon memory move within `realloc()` not guaranteed? – Qix - MONICA WAS MISTREATED May 09 '17 at 09:49
  • Yes, the memory will be freed but that usually just means it goes on a list of free blocks of memory. It will not be removed from the process's memory map which means that there will be no hardware checks on reads and writes and therefore no segmentation fault. – JeremyP May 09 '17 at 09:53
2

The problem is that with the addValue, it calls realloc but that value is only local to the function. You update the value of nA by passing it by reference, so you could either do that with A or have the function return the local version.

For example to do the second you change addValue to return int * and have it return the local value of A

int *addValue(int *A, int *n, int Data)
{
     A = realloc(A, (*n+1)*sizeof(*A));
     A[*n] = Data;
     (*n)++;
     return(A);
}

then when you call it you assign the return value to A like this

A=addValue(A,&nA,temp);

then, A will update each time you call that function.

It was also using the wrong datatype (int *) in the sizeof when it should have been int so I've also corrected that. By using *A, no matter what datatype A is, it'll always allocate the right size of memory.

Chris Turner
  • 8,082
  • 1
  • 14
  • 18
  • Could you give me a sample code show how to fix this problem? I still don't understand you clearly. – Dang_Ho May 09 '17 at 09:27
  • @Dang_Ho Does that help? You already know how to pass by reference because you're doing it with `nA` so I included how to do it the other way. – Chris Turner May 09 '17 at 09:34
  • @Dang_Ho there was also another problem with the way you calling `realloc` namely the wrong datatype being passed to `sizeof` which I've corrected in the example code. – Chris Turner May 09 '17 at 09:57
0

reallocated pointer stays in local copy of A in addValue

Variable A from main has only one int allocated when you go to print, loop goes on accessing till A[15] leading to undefined behaviour

Pras
  • 4,047
  • 10
  • 20