Quick question about C memory management

Discussion in '3DS - Homebrew Development and Emulators' started by mashers, Oct 29, 2015.

  1. mashers
    OP

    mashers Stubborn ape

    Member
    3,837
    5,155
    Jun 10, 2015
    Kongo Jungle
    Hey guys,

    Objective C has spoiled me, especially with ARC, so I need to double check something on C memory management. Take the following example:

    Code:
    typedef struct myStruct {
        u8 * spriteData;
    } myStruct;
    
    myStruct myStructArray[16];
    
    void doStuffWithStruct(int i) {
        myStruct aStruct = myStructArray[i];
        u8 * data = malloc(1024);
        doStuffWithData(data);
        aStruct.spriteData = data;
    }
    
    int main() {
        doStuffWithStruct(0);
        doStuffWithStruct(0);
    }
    
    I'm assuming that the second call to doStuffWithStruct(0) will cause aStruct.spriteData to leak. I'm wondering about two possible solutions.

    Solution 1
    Code:
    typedef struct myStruct {
        u8 * spriteData;
    } myStruct;
    
    myStruct myStructArray[16];
    
    void doStuffWithStruct(int i) {
        myStruct aStruct = myStructArray[i];
        u8 * data = malloc(1024);
        doStuffWithData(data);
        aStruct.spriteData = data;
    
    /*
    Free data here. This is like retaining properties in Obj-C, 
    but I'm concerned aStruct.spriteData will then point to 
    NULL or garbage in this instance
    */
    
        free(data);
    }
    
    int main() {
        doStuffWithStruct(0);
        doStuffWithStruct(0);
    }
    
    Solution 2
    Code:
    typedef struct myStruct {
        u8 * spriteData;
    } myStruct;
    
    myStruct myStructArray[16];
    
    void doStuffWithStruct(int i) {
        myStruct aStruct = myStructArray[i];
    
    /*
    Free data here if it already exists. Is the if statement needed 
    to prevent attempting to free NULL if aStruct.spriteData has 
    not yet been allocated, or is freeing a NULL pointer ok?
    */
        if (aStruct.spriteData) {
            free(aStruct.spriteData);
        }
    
        u8 * data = malloc(1024);
        doStuffWithData(data);
        aStruct.spriteData = data;
    }
    
    int main() {
        doStuffWithStruct(0);
        doStuffWithStruct(0);
    }
    

    So, which, if either, of these will prevent a memory leak? Also, do I need to go through and free the memory for all of the myStruct objects in myStructArray before terminating the application, or will that be taken care of for me when the array itself is freed?
     
    Last edited by mashers, Oct 29, 2015
  2. TheCruel

    TheCruel Developer

    Banned
    1,351
    2,884
    Dec 6, 2013
    United States
    You should be able to free a null pointer:

    But never free the pointer if you plan on using the data sometime later.
     
  3. Gocario

    Gocario GBAFail'd

    Member
    640
    560
    Sep 5, 2015
    France
    Bourg Palette
    The first solution will always free the memory within the function (= no leak IF the pointer data wasn't malloced BEFORE).
    The second will need a free function further.


    Code:
    // If ptr is a null pointer, no action occurs.
    void free(void*ptr);
    
    Care:
    • Once freed, spriteData will still be pointing to the address returned by the previous malloc. You should set it manually to NULL after the free.
    • If spriteData isn't initialized to NULL, problems might appear within `if (.spriteData)`.
     
  4. mashers
    OP

    mashers Stubborn ape

    Member
    3,837
    5,155
    Jun 10, 2015
    Kongo Jungle
    Great, thanks. So solution 2 is the right one since I don't need the old content of spriteData. Is that right?
     
  5. TheCruel

    TheCruel Developer

    Banned
    1,351
    2,884
    Dec 6, 2013
    United States
    Yeah, most likely. Definitely not the first one.
     
  6. mashers
    OP

    mashers Stubborn ape

    Member
    3,837
    5,155
    Jun 10, 2015
    Kongo Jungle
    Oh right. So I should do

    free(aStruct.spriteData);
    aStruct.spriteData = NULL;
     
  7. Gocario

    Gocario GBAFail'd

    Member
    640
    560
    Sep 5, 2015
    France
    Bourg Palette
    I am pointing this again, with this link.

    PS: What is the default value when declaring a pointer, NULL or not set?
     
  8. doctorgoat

    doctorgoat GBAtemp Advanced Fan

    Member
    624
    234
    Jun 3, 2015
    United States
    God, I hate not having garbage collection. ._.
     
  9. Gocario

    Gocario GBAFail'd

    Member
    640
    560
    Sep 5, 2015
    France
    Bourg Palette
    Garbage collectors are bad in this case... :c
    I don't like them in this case!
     
    Last edited by Gocario, Oct 30, 2015
  10. mashers
    OP

    mashers Stubborn ape

    Member
    3,837
    5,155
    Jun 10, 2015
    Kongo Jungle
    I found that I got lock ups when freeing the pointer if I didn't check it was NULL first. Perhaps free() works differently in ctrulib.
     
  11. sarkwalvein

    sarkwalvein Professional asshole at GBATemp

    Member
    GBAtemp Patron
    sarkwalvein is a Patron of GBAtemp and is helping us stay independent!

    Our Patreon
    5,011
    5,196
    Jun 29, 2007
    Germany
    Niedersachsen
    Does your example work?
    There are many privilege.
    First when you write:
    "myStruct aStruct = myStructArray;"
    You are creating a copy, not a reference.
    That copy and all modifications to it will die within the function.

    Oh... I will look for a computer to finish this reply, the phone sucks.
     
  12. Gocario

    Gocario GBAFail'd

    Member
    640
    560
    Sep 5, 2015
    France
    Bourg Palette
    He is using pointer to reference infact: https://github.com/mashers/3ds_hb_menu/blob/master/source/themegfx.c#L65
     
  13. sarkwalvein

    sarkwalvein Professional asshole at GBATemp

    Member
    GBAtemp Patron
    sarkwalvein is a Patron of GBAtemp and is helping us stay independent!

    Our Patreon
    5,011
    5,196
    Jun 29, 2007
    Germany
    Niedersachsen
    Oh, okay. I was replying based only in the posted example.
    Another thing to mention is that free doesn't make the pointer NULL.
    So if somebody wants to "check for NULL" in order to assign memory, it is compulsory to set the pointer to null after free manually:
    free(pointer);
    pointer=NULL;

    So:
    if(aStruct->spriteData){
    free(aStruct->spriteData);
    aStruct->spriteData = 0;
    }

    ** But I see this is already being done in the github also.
     
    Last edited by sarkwalvein, Oct 30, 2015
  14. Gocario

    Gocario GBAFail'd

    Member
    640
    560
    Sep 5, 2015
    France
    Bourg Palette
    As he said:
     
  15. mashers
    OP

    mashers Stubborn ape

    Member
    3,837
    5,155
    Jun 10, 2015
    Kongo Jungle
    Heh, you realised which function I was talking about ;) I thought the real function was too complex for this questions so I simplified with an example, but got the referencing wrong in the example!

    And yes, I'm setting it to NULL manually now. The gridlauncher is probably so littered with uninitialised variables that it's a miracle it works at all. I've got so used to not having to do this in Obj-C, since everything is initialised to nil, 0 or false automatically when the variable is created.

    Is there a static analyser which works with ctrulib? It would be good to run the code through something like clang to check for problems like this.

    — Posts automatically merged - Please don't double post! —

    Oh and one other question, do I need to go through the array and release any existing objects or will that be taken care of for me?
     
  16. Gocario

    Gocario GBAFail'd

    Member
    640
    560
    Sep 5, 2015
    France
    Bourg Palette
    Analysers? What are they? B-)
    > Everytime you use a malloc, a free shall be used.
     
  17. mashers
    OP

    mashers Stubborn ape

    Member
    3,837
    5,155
    Jun 10, 2015
    Kongo Jungle
    Great, thank you. Time to search the source folder for instances of the words "malloc" and "calloc"...
     
  18. sarkwalvein

    sarkwalvein Professional asshole at GBATemp

    Member
    GBAtemp Patron
    sarkwalvein is a Patron of GBAtemp and is helping us stay independent!

    Our Patreon
    5,011
    5,196
    Jun 29, 2007
    Germany
    Niedersachsen
    You take care of everything memory related.
    If you created an array that contains pointers, and then assigned memory to those pointers, you have to use free for each one of those pointers later on.
     
  19. mashers
    OP

    mashers Stubborn ape

    Member
    3,837
    5,155
    Jun 10, 2015
    Kongo Jungle
    Thank you for clarifying. This is different to what I'm used to in Obj-C, even using manual reference counting. Take for example:

    Code:
    NSString *s1 = [[NSString alloc] initWithString:@"Hello];
    NSString *s2 = [[NSString alloc] initWithString:@"World];
    NSArray *a = [[NSArray alloc] initWithObjects:s1, s2, nil];
    
    To release everything, all I have to do is call [a release]. This will decrement the retain count of a to 0 causing its dealloc function to be called. [NSArray dealloc] overloads [NSObject dealloc] to iterate through its contents and call release on each object. This causes dealloc to be called on each item within the array, and everything gets deallocated. Of course with ARC you don't have to do anything - just alloc and let the runtime deal with when things get deallocated.

    So yeah, long story short, having to loop through an array and manually free anything which might have been allocated feels weird to me (but does make sense).
     
  20. filfat

    filfat Musician, Developer & Entrepreneur

    Member
    1,229
    858
    Nov 24, 2012
    Super OT,
    but Objective C looks like someone combined C# with a ini config file :P