free(ptr) crashes sometimes, pointer is always valid

Question

I am trying to create some streams of bytes, dynamically allocated, and perform a copy of them in some other place. My code is this (earlier i didn`t type from a pc :) ):

    void construct_cypherstreams(uint8_t * stream, int key_length, int stream_length, uint8_t ** encr_streams, int * bytes_in_stream) {
        // chyperstream = the stream formed of every ith byte
        uint8_t * cypherstream;
        int length;
        length = stream_length / key_length + 1;
        // each byte of the key can have values
        // between 0 and 256
        int i = 0;
        int num_added = 0;
        for (int k = 0; k < key_length; k++) {
            printf("\n%s %d\n", "iteration", k);
            i = k; num_added = 0;
            cypherstream = (uint8_t *)malloc(length * sizeof (char));
            if (cypherstream == NULL) {
                printf("%s\n", "could not allocate");
                exit(1);
            }
            else {
                printf("%s\n", "succesfully allocated");
            }
            while (i < stream_length) {
                // construct cypherstream
                cypherstream[num_added] = stream[i];
                num_added++;
                i += key_length;
            }
            printf("\n%s\n", "created cypherstream:");
            for (int m = 0; m < num_added; m++) {
                printf("%X", cypherstream[m]);
            }
            printf("\n");
            printf("%s\n", "making deep copy...");
            encr_streams[k] = (uint8_t *)malloc(num_added * sizeof(char));
            // perform deep copy of characters
            for (int n = 0; n < num_added; n++) {
                encr_streams[k][n] = cypherstream[n];
            }
            printf("%s\n", "done making deep copy");
            free(cypherstream);
            printf("%s\n", "succesfully freed");
            printf("%s %d\n", "position:", k);
            printf("%s %d\n", "num_added:", num_added);
            bytes_in_stream[k] = num_added;
            printf("%s\n", "iteration ended");
        }
    }

And I call it like this:

    uint8_t ** encr_streams;
    int  * bytes_in_stream;
    encr_streams = (uint8_t **)malloc(key_length * sizeof **encr_streams);
    bytes_in_stream = (int *)malloc(key_length * sizeof *bytes_in_stream);
    construct_cypherstreams(stream, key_length, stream_length, encr_streams, bytes_in_stream);

Now my program sometimes runs, sometimes crashes. I am stuck here for the moment and I could really use some help. Compiler: msvc Thanks

If I can't "argue" about casting pointers, can I point out that your `printf()` usage is super-strange? You could just do `printf("making deep copy...\n");` or `puts("making deep copy...");` instead of that strangeness. — unwind, Nov 26 '14 at 11:34
Make sure you haven't done `stream++` or any other arithmetic operations on `stream` pointer in `//do a lot of stuff`. — Santosh A, Nov 26 '14 at 11:34
It's not enough for a pointer to be valid. The argument of `free` must be the exact value returned from a call of `malloc`/`calloc`/`realloc`. — Kerrek SB, Nov 26 '14 at 11:36
No arithmetic, i only use indexes. It's also the same value returned by malloc. No realloc or calloc — Hame, Nov 26 '14 at 11:37
to add to previous comments, please make sure, you're not `free()`-ing `stream` already somewhere inside `//lot of sutff`. — Sourav Ghosh, Nov 26 '14 at 11:46
You're not showing the relevant code, so it's not possible to help you. The most likely error is that you overwrite either `stream`, or some other dynamically allocated memory. Doing such overwrites can corrupt the heap system's internal book-keeping data, which causes `free()` to crash when it relies on that data to be correct. — unwind, Nov 26 '14 at 11:36

score 3 · Answer 1 · edited Nov 26 '14 at 19:48

3

This is a case of heap corruption as you are trying to overwrite some dynamically allocated memory.

edited Nov 26 '14 at 19:48

animuson

53,861
28
137
147

answered Nov 26 '14 at 11:51

Ben Williams

109
2
11

can you please explain your answer? – Hame Nov 29 '14 at 07:44

score 3 · Accepted Answer · answered Nov 26 '14 at 12:58

3

encr_streams = (uint8_t **)malloc(key_length * sizeof **encr_streams);

just looks wrong. I think it should be

encr_streams = malloc(key_length * sizeof *encr_streams);

because you seem to intend to allocate an array of pointers to uint8_t. Then you probably also have to initialize the elements of that array by something.

answered Nov 26 '14 at 12:58

Jens Gustedt

76,821
6
102
177

i actually want to allocate an array of pointers. – Hame Nov 26 '14 at 13:47
You were right. I actually made it work after I changed the code as Ben Williams suggested, but I guess I had more than 1 problem. I realized I wanted an arary of pointers but I was allocating space for `char`s. I guess I was just lucky it worked – Hame Nov 28 '14 at 12:04

score 1 · Answer 3 · edited May 23 '17 at 12:16

1

If you program on Linux, run your code under the valgrind memory debugger:

http://valgrind.org/docs/manual/quick-start.html

For Windows... you may want to try MS AppVerifier though I haven't used it for years and forgot almost everything about it :-(

How to use Microsoft Application Verifier

edited May 23 '17 at 12:16

Community

1
1

answered Nov 26 '14 at 12:02

nodakai

7,773
3
30
60

free(ptr) crashes sometimes, pointer is always valid

3 Answers3