3

This is from gdb:

22      database->size = size;
(gdb) n
23      return database;
(gdb) p size
$6 = 1401
(gdb) p database->size
$7 = 3086862424
(gdb) p &size
$8 = (unsigned int *) 0xbffff050
(gdb) p &database->size
$9 = (unsigned int *) 0xb7fc6ff8

This is from the code:

typedef struct _DATABASE {
    RESULT* res;
    unsigned int size;
} DATABASE;

....
....

DATABASE* alloc_database(unsigned int size, DATABASE* database)
{
    database = (DATABASE*) malloc (sizeof(DATABASE));
    if (!database) return NULL;
    database->res = (RESULT*) malloc (sizeof(RESULT) * size);
    if (!database->res) {
        free_database(database);
        return NULL;
    }
    memset(database->res, 0, sizeof(RESULT) * size);
    database->size = size;
    return database;
}

You can see that both database->size and size are from the (unsigned int) type, in both code and gdb, but for some reason, after the assignment the values are different.

Does anyone knows the what is the reason of that?

7
  • 3
    Ever heard of calloc()? It does allocate and zero in one function call — and can use optimally efficient zeroing since it knows the data is aligned where memset() does not. Dec 27, 2012 at 21:38
  • 1
    Are you sure your code is compiled without optimization ?
    – nos
    Dec 27, 2012 at 21:39
  • what does the calling function look like?
    – old_timer
    Dec 27, 2012 at 21:40
  • @JonathanLeffler: I don't think alignment is where calloc excels; the only place misalignment overhead matters in memset is for tiny values of n where the startup overhead dominates. For large n that gets lost in the noise. The real performance advantage of calloc is that it can sometimes know the memory is pre-zeroed (e.g. when obtained via a new mmap) and thus avoid the zero-fill operation entirely. Dec 27, 2012 at 21:42
  • 2
    Yes, same remark as Jonathan. Use calloc but also use the appropriate type size_t instead of unsigned int. This will make the program more portable. You don't need to cast the return from malloc in C either. There's also a potential problem in the unlikely case your 2nd malloc fails, you call free_database() with an uninitialised structure, that can be problematic. Dec 27, 2012 at 21:43

1 Answer 1

1

database is local to the function alloc_database. You assign to it the result of a malloc, but this assignment is local to the function. After return, database returns to the value it had when the function was called. Note that in gdb, you inspect the value of database->size, AFTER the return. So you inspect it in a scope where the value of database is outside the function.

You have two options:

  1. Change the function to receive only the size argument, allocate to a local and return it. Then you can assign the return value and check it in gdb:

  2. If you want to return a result in the database argument, you need to pass a pointer to the database pointer.

This is the code for option 2:

DATABASE* alloc_database(unsigned int size, DATABASE** database)
{
    *database = (DATABASE*) malloc (sizeof(DATABASE));
    if (! *database) return NULL;
    (*database)->res = (RESULT*) malloc (sizeof(RESULT) * size);
    if (!(*database)->res) {
        free_database((database);
        *database = NULL;
        return NULL;
    }
    memset((*database)->res, 0, sizeof(RESULT) * size);
    (*database)->size = size;
    return (*database);
}

P.S. אהבה לא באה בחינם...

5
  • Well, if it's passed back as a return value, it should be fine. If the calling code is expecting it to be filled in, your solution is good [I tried editing it to make the layout right, but somehow, it doesn't like it. I will have another go. [And again, the problem is quite possibly in the code we don't see - 80% rule applies] Dec 27, 2012 at 21:52
  • Just a further note. It makes little sense to BOTH pass in a pointer to pointer, AND return the database pointer. Dec 27, 2012 at 21:53
  • +1 for the spotting the right problem and proffering up a solution. The OP may well repeatedly call this with the same DB pointer, and if so, it (and they by inference) will still leak memory, just as it would have prior. You can only do so much to protected someone from themselves though. PS: I prefer out-params to return results as well.
    – WhozCraig
    Dec 27, 2012 at 21:58
  • @grebulon. 'Note that in gdb, you inspect the value of database->size, AFTER the return' No, he's still in the frame of alloc_database().
    – fizzer
    Dec 27, 2012 at 21:58
  • grebulon, thanks for your reply and solution. I've had few issues with my code and this was one of them. As fizzer mentioned, The gdb scope was the same as the one where I assigned the value to database->size (i.e., in alloc_database()), and this was the reason for me to post this question. Sorry for not being clear on that. I've already re-wrote my code from scratch, so I no longer have issues with it, but the question still remains... P.S. תתפלא :) Jan 2, 2013 at 17:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.