1

I already know that there is no way to know if a pointer target still a valid allocation of it's already freed, so I'm trying to use pointer to pointer to solve this but it didn't work.

My objective is simply making print_block() detect if block pointer is Null or not.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void free_block(u_int8_t **_block) {

    // Dereference
    u_int8_t *block = *_block;

    // Info
    printf("free_block()\t-> %p Point To %p \n", &block, block);

    // Free Block
    free(block);
    block = NULL;
}

void print_block(u_int8_t **_block) {

    // Dereference
    u_int8_t *block = *_block;

    // Detectc if this block is freed
    // This is the objective of this code
    if(block == NULL) {

        printf("print_block()\t-> %p Is Null.\n", block);
        return;
    }

    // Info
    printf("print_block()\t-> %p Point To %p -> ", &block, block);

    // Print byte by byte
    u_int8_t *p = block;
    for(int i = 0; i < 3; i++) {

        printf("0x%02X ", *(u_int8_t *)p);
        p++;
    }
    printf("\n");
}

int main(void) {

    // Allocat a block in the memory
    u_int8_t *block = malloc(3 * sizeof(u_int8_t));

    // Set all to zeros
    memset(block, 0x00, 3);

    // Info
    printf("Main()\t\t\t-> %p Point To %p \n", &block, block);

    // Print the block content
    print_block(&block);

    // Free the block
    free_block(&block);

    // Print the block content gain
    // This shold print Null because
    // we freed the block.
    print_block(&block);

    return 0;
}

Result

Main()          -> 0x7fffd549cc58 Point To 0xfa42a0 
print_block()   -> 0x7fffd549cc28 Point To 0xfa42a0 -> 0x00 0x00 0x00 
free_block()    -> 0x7fffd549cc60 Point To 0xfa42a0 
print_block()   -> 0x7fffd549cc28 Point To 0xfa42a0 -> 0xA4 0x0F 0x00 
ShellX3
  • 51
  • 8
  • 10
    `block = NULL;` should be `*_block = NULL;`. `block = NULL;` does nothing because `block` is about to go out of scope. – David Schwartz May 04 '22 at 19:45
  • 1
    Further, `printf("free_block()\t-> %p Point To %p \n", &block, block);` is effectively worthless for the first argument, `&block`. Who cares about the address of a local variable? Those two arguments should be `_block`, and `*_block`. Honestly the usefulness of `block` in that function *at all* is debatable. – WhozCraig May 04 '22 at 19:53
  • Note that you should not, in general, create function, variable, tag or macro names that start with an underscore. Part of [C11 §7.1.3 Reserved identifiers](https://port70.net/~nsz/c/c11/n1570.html#7.1.3) says: — _All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use._ — _All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces._ See also [What does double underscore (`__const`) mean in C?](https://stackoverflow.com/q/1449181) – Jonathan Leffler May 04 '22 at 19:55
  • You might take a look at Steve Maguire [Writing Solid Code: 20th Anniversary 2nd Edn](https://smile.amazon.com/dp/1570740550) 2013. There are those who intensely dislike this book; I think it is quite useful. It includes code that wraps memory management functions so that you can track whether a given pointer is still valid, and discusses some of the pitfalls of using it. Notably, you have to arrange to wrap any function that allocates memory — `strdup()`, for example — and make sure you use the wrapper in your code. – Jonathan Leffler May 04 '22 at 19:58
  • What problem are you trying to solve, that a memory debugger doesn't already do? – Cheatah May 04 '22 at 20:24
  • @DavidSchwartz, Straight to the point, that exactly what I was missing, Thank you. – ShellX3 May 05 '22 at 12:45
  • @WhozCraig, True, I agree, I completely removed `block` now. Thank you. – ShellX3 May 05 '22 at 12:46
  • @JonathanLeffler, I taught we should avoid the double underscore only, I didn't know the single underscore is also reserved, I removed all underscores now, Thank you. – ShellX3 May 05 '22 at 12:49

2 Answers2

0

There is no standard way to tell if a pointer is valid, nor if it points to an allocated object nor if it was freed already.

Yet it is possible to design a framework on top of the allocation functions to keep track of all allocations and deallocations and maintain a list of valid pointers. Looking up a pointer value in this list will tell you if the pointer points to a valid allocated block and possibly what size was requested from the heap.

Here is a simplistic implementation:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void track_add_pointer(void *p, size_t size);
int track_find_pointer(void *p, size_t *sizep);
void track_remove_pointer(void *p);

void *tmalloc(size_t size) {
    void *p = (malloc)(size);
    if (p) {
        track_add_pointer(p, size);
    }
    return p;
}

void *tcalloc(size_t size, size_t nmemb) {
    void *p = (calloc)(size, nmemb);
    if (p) {
        track_add_pointer(p, size * nmemb);
    }
    return p;
}

void *trealloc(void *p, size_t size) {
    if (p) {
        void *newp = NULL;
        if (!track_find_pointer(p, NULL)) {
            fprintf(stderr, "trealloc: invalid pointer %p\n", p);
        } else {
            if (size == 0) {
                (free)(p);
            } else {
                newp = (realloc)(p, size);
            }
            if (newp != p) {
                if (p) track_remove_pointer(p);
                if (newp) track_add_pointer(newp);
            }
        }
        return newp;
    } else {
        return tmalloc(size);
    }
}

void tfree(void *p) {
    if (p) {
        if (!track_find_pointer(p, NULL)) {
            fprintf(stderr, "tfree: invalid pointer %p\n", p);
        } else {
            (free)(p);
            track_remove_pointer(p);
        }
    }
}

char *tstrdup(const char *s) {
    char *p = NULL;
    if (s) {
        size_t len = strlen(s);
        p = tmalloc(len + 1);
        if (p) {
            memcpy(p, s, len + 1);
        }
    } else {
        fprintf(stderr, "tstrdup: null pointer\n");
    }
    return p;
}

char *tstrndup(const char *s, size_t n) {
    char *p = NULL;
    if (s) {
        size_t len = strnlen(s, n);
        p = tmalloc(len + 1);
        if (p) {
            memcpy(p, s, len);
            p[len] = '\0';
        }
    } else {
        fprintf(stderr, "tstrndup: null pointer\n");
    }
    return p;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Thank you for your explanation, but I guess `if(p)` it's not a good practice, this is a prove code https://godbolt.org/z/dMb4ePrhn. Please correct me if I'm wrong. – ShellX3 May 05 '22 at 13:21
  • it's should be `if(p != NULL)` – ShellX3 May 05 '22 at 13:23
  • never mind, I played more with your code and it's seem `if(p)` success too like `if(p != NULL)`, I still don't know what is the best practice :) – ShellX3 May 05 '22 at 14:32
  • @ShellX3: `if (p)` and `if (p != NULL)` are exactly equivalent and both are idiomatic in C. There is no *best practice*, but there might be local conventions for a given project or inside a company that recommend one or the other, but it is really a matter of style and personal choice. I personally prefer the conciseness of `if (p)`. It is also important to be consistent: avoid mixing styles in the same project. – chqrlie May 05 '22 at 19:49
  • I got it, it's true, even a ASM generated code seem almost the same. – ShellX3 May 05 '22 at 19:58
  • I'm trying to do the same thing in a multi-threading app, seem a little bit challenging.. can you please look at this https://replit.com/@ShellX3/PointerMultiThreading#main.c ? – ShellX3 May 05 '22 at 19:59
  • Your test does not work: `block` is a local variable in both the `main` function and the `my_thread` function. These variables are distinct. Setting `block` to `NULL` in `main()` has no effect on the variable `block` in the running `my_thread()` function. Accessing the allocated block from both threads without a sequencing mechanism has undefined behavior and so does freeing the block while the other thread still accesses it. – chqrlie May 05 '22 at 21:02
  • Understood, finally I did something similar like your `track_add_pointer()` example, and it's working good now, Thank you for all your explanations. – ShellX3 May 05 '22 at 21:07
0

You can use a Block struct and a bit of discipline.

As an example

typedef struct
{
    size_t   size;
    uint8_t  fill;
    uint8_t* data;

} Block;

Block* free_block(Block*);
Block* get_block(size_t size, uint8_t fill_value);
void   print_block(Block* block, const char* title);

and

  • make the functions operate on pointers to Block, encapsulating the data
  • make free_block() return NULL to ease the task of invalidating the block pointer
  • test the block address inside print_block()
  • use an optional title in print_block()

main as an example

int main(void)
{
    const int test_size = 32;
    Block* block = get_block(test_size, 0);  // zero-filled
    printf(
        "Main()\t\t\t-> Pointer at %p points to data at "
        "%p\n",
        &block, block->data);
    print_block(block, "\nBlock contents:");
    block = free_block(block);
    print_block(block, "\nafter free()");
    return 0;
}

Maybe you find this way easier to read.

output of main as above

Main()                  -> Pointer at 00CFF754 points to data at 00E96C70

Block contents
print_block()-> valid block data of size 32 at 00E96C70
                fill char is 00

00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

after free()
print_block()   -> block is null.

complete code

#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct
{
    size_t   size;
    uint8_t  fill;
    uint8_t* data;

} Block;

Block* free_block(Block*);
Block* get_block(size_t size, uint8_t fill_value);
void   print_block(Block* block, const char* title);

int main(void)
{
    const int test_size = 32;
    Block* block = get_block(test_size, 0);  // 3 bytes, zero filled
    printf(
        "Main()\t\t\t-> Pointer at %p points to data at "
        "%p\n",
        &block, block->data);
    print_block(block, "\nBlock contents:");
    block = free_block(block);
    print_block(block, "\nafter free()");
    return 0;
}

Block* free_block(Block* block)
{
    if (block == NULL) return NULL;
    if (block->data == NULL) return NULL;
    free(block->data);  // data is free
    free(block);
    return NULL;
}

Block* get_block(size_t size, uint8_t fill)
{
    Block* block = malloc(sizeof(Block));
    if (block == NULL) return NULL;
    block->data = malloc(size * sizeof(uint8_t));
    if (block->data == NULL) return NULL;
    memset(block->data, fill, size);
    block->fill = fill;
    block->size = size;
    return block;
}

void print_block(Block* block, char* title)
{
    if (title != NULL) printf("%s\n", title);
    if (block == NULL)
    {
        printf("print_block()\t-> block is null.\n");
        return;
    }
    printf(
        "print_block()->\tvalid block data of size %d at "
        "%p\n\t\tfill char is %02X\n\n",
        block->size, block->data, block->fill);
    // Print byte by byte
    const int nc = 16; // 16 bytes per line
    size_t    j  = 1;
    for (size_t i = 0; i < block->size; i += 1)
    {
        printf("%02X ", block->data[i]);
        if (j >= nc)
            printf("\n"), j = 1;
        else
            j += 1;
    }
    if ( j != 1 ) printf("\n");
    return;
}
arfneto
  • 1,227
  • 1
  • 6
  • 13
  • 1
    Your code solve the issue which is catching `block is null`, no pointer-to-pointer is needed, and I can use it in a multi-threading environment, thank you :) – ShellX3 May 05 '22 at 13:43
  • I just discovered that doing this in multi-threading app is another animal.. I tried this https://replit.com/@ShellX3/PointerMultiThreading#main.c – ShellX3 May 05 '22 at 19:45
  • you will need to have some kind of lock to protect the pointers for it to work in multi-threaded apps – arfneto May 05 '22 at 20:54
  • I did something similar like @chqrlie `track_add_pointer()` example, and it's working good now, Thank you for all your explanations. – ShellX3 May 05 '22 at 21:09