Fix memory errors, improve enum. impl.

DOCUMENTATION.md

@@ -113,7 +113,7 @@ bool impl_foo_command(light_device_target_t *target, char const *command_string)
 
 In the sourcefile, you need to implement the 6 methods. Make sure to return `true` on success and `false` on failure. If you do not actually implement a function (for example `impl_foo_command`), just return `true`.
 
-The job of the enumerator is to identify/enumerate a bunch of different devices (or just one, or even zero if it doesnt find any). You are also responsible to create the device targets for them (i.e, the things that you actually write to on the device). You do this by setting the devices and targets up in `impl_foo_init` and to free these in `impl_foo_free`. The enumerator (this code) owns the memory of the devices and targets, and thus is in charge of allocating and freeing it. 
+The job of the enumerator is to identify/enumerate a bunch of different devices (or just one, or even zero if it doesnt find any). You are also responsible to create the device targets for them (i.e, the things that you actually write to on the device). You do this by setting the devices and targets up in `impl_foo_init`. You are not required to do anything in `impl_foo_free`, any allocated memory will be automatically free'd by light, including device/target data that you allocate yourself. You may use `impl_foo_free` to free resources you allocate outside of the light API.
 
 ```c
 
@@ -126,83 +126,26 @@ bool impl_foo_init(light_device_enumerator_t *enumerator)
 {
     /* Lets create a single device, with a single target, for simplicity */
     
-    /* Allocate a new device */
-    light_device_t *new_device = malloc(sizeof(light_device_t));
+    /* Create a new device called new_device_name, we dont need any userdata so pass NULL to the device_data parameter */
+    light_device_t *new_device = light_create_device(enumerator, "new_device_name", NULL)
     
-    /* Set the name of the new device to "new_device_name" */
-    snprintf(leds_device->name, sizeof(leds_device->name), "%s", "new_device_name");
-    
-    /* Add the newly created device to the enumerator */
-    light_add_enumerator_device(enumerator, new_device);
-
-    
-    
-    /* Allocate a new device target for the newly created device */
-    light_device_target_t *new_target = malloc(sizeof(light_device_target_t));
-    
-    /* Set the name of the new target */
-    snprintf(new_target->name, sizeof(new_target->name), "%s", "new_target_name");
-    
-    /* Setup the function bindings for this target, i.e. what functions will run internally when you run different commands on this device target */
-    new_target->set_value = impl_foo_set;
-    new_target->get_value = impl_foo_get;
-    new_target->get_max_value = impl_foo_getmax;
-    new_target->custom_command = impl_foo_command;
-    
-    /* Finally add the target to the device */
-    light_add_device_target(new_device, new_target);
-    
-    
-    
-    /* Optional: Setup data specific to either a device or a target, or both. */ 
+    /* Setup userdata specific to the target we will create*/ 
     /* Useful to for example reference an ID in a third-party API or likewise */
+    /* NOTE: The userdata will be free()'d automatically on exit, so you do not need to free it yourself */
     impl_foo_data_t *custom_data = malloc(sizeof(impl_foo_data_t));
     custom_data->internal_quack_id = 333;
     
-    /* You can set it to the device itself, or to a target */
-    new_device->device_data = custom_data;
-    new_target->device_target_data = custom_data;
-
+    
+    /* Create a new device target called new_target_name, and pass in the functions and userdata that we just allocated */
+    light_create_device_target(new_device, "new_target_name", impl_foo_set, impl_foo_get, impl_foo_getmax, impl_foo_command, custom_data)
+    
     /* Return true because we didnt get any errors! */
     return true;
 }
 
 bool impl_foo_free(light_device_enumerator_t *enumerator)
 {
-    /* We are responsible to free the memory we allocated in init, so lets do that */
-    
-    /* Iterate through the devices in the enumerator */
-    for(uint64_t d = 0; d < enumerator->num_devices; d++)
-    {
-        light_device_t *curr_device = enumerator->devices[d];
-
-        /* Iterate throug hthe targets in the device */
-        for(uint64_t t = 0; t < curr_device->num_targets; t++)
-        {
-            light_device_target_t *curr_target = curr_device->targets[t];
-            
-            /* If we allocated any target data, free it here */
-            if(curr_target->device_target_data != NULL)
-            {
-                free(curr_target->device_target_data);
-            }
-            
-            free(curr_target);
-        }
-
-        /* If we allocated any device data, free it here */
-        if(curr_device->device_data != NULL)
-        {
-            free(curr_device->device_data);
-        }
-        
-        /* We need to 'dispose' of the device when we are done with it, to free some internal memory. Always do this when you are about to free a device that you allocated. */
-        light_dispose_device(curr_device);
-        
-        /* Free the device */
-        free(curr_device);
-    }
-    
+    /* We dont need to do anything here, but if we want to, we can free some third-party API resources */
     return true;
 }
 

src/impl/sysfs.c

@@ -10,12 +10,8 @@
 static bool _impl_sysfs_init_leds(light_device_enumerator_t *enumerator)
 {
     // Create a new backlight device
-    light_device_t *leds_device = malloc(sizeof(light_device_t));
-    snprintf(leds_device->name, sizeof(leds_device->name), "%s", "leds");
-    
-    // Add it to the enumerator 
-    light_add_enumerator_device(enumerator, leds_device);
-    
+    light_device_t *leds_device = light_create_device(enumerator, "leds", NULL);
+
     // Iterate through the led controllers and create a device_target for each controller 
     DIR *leds_dir;
     struct dirent *curr_entry;
@@ -34,24 +30,13 @@ static bool _impl_sysfs_init_leds(light_device_enumerator_t *enumerator)
             continue;
         }
         
-        // Create a new device target for the controller 
-        light_device_target_t *led_controller = malloc(sizeof(light_device_target_t));
-        snprintf(led_controller->name, sizeof(led_controller->name), "%s", curr_entry->d_name);
-        
-        // Setup the function bindings
-        led_controller->set_value = impl_sysfs_set;
-        led_controller->get_value = impl_sysfs_get;
-        led_controller->get_max_value = impl_sysfs_getmax;
-        led_controller->custom_command = impl_sysfs_command;
-        
         // Setup the target data 
         impl_sysfs_data_t *dev_data = malloc(sizeof(impl_sysfs_data_t));
-        led_controller->device_target_data = dev_data;
         snprintf(dev_data->brightness, sizeof(dev_data->brightness), "/sys/class/leds/%s/brightness", curr_entry->d_name);
         snprintf(dev_data->max_brightness, sizeof(dev_data->max_brightness), "/sys/class/leds/%s/max_brightness", curr_entry->d_name);
         
-        // Add it to the device
-        light_add_device_target(leds_device, led_controller);
+        // Create a new device target for the controller 
+        light_create_device_target(leds_device, curr_entry->d_name, impl_sysfs_set, impl_sysfs_get, impl_sysfs_getmax, impl_sysfs_command, dev_data);
     }
     
     closedir(leds_dir);
@@ -62,12 +47,8 @@ static bool _impl_sysfs_init_leds(light_device_enumerator_t *enumerator)
 static bool _impl_sysfs_init_backlight(light_device_enumerator_t *enumerator)
 {
     // Create a new backlight device
-    light_device_t *backlight_device = malloc(sizeof(light_device_t));
-    snprintf(backlight_device->name, sizeof(backlight_device->name), "%s", "backlight");
-    
-    // Add it to the enumerator 
-    light_add_enumerator_device(enumerator, backlight_device);
-    
+    light_device_t *backlight_device = light_create_device(enumerator, "backlight", NULL);
+
     // Iterate through the backlight controllers and create a device_target for each controller 
     DIR *backlight_dir;
     struct dirent *curr_entry;
@@ -90,22 +71,14 @@ static bool _impl_sysfs_init_backlight(light_device_enumerator_t *enumerator)
             continue;
         }
         
-        // Create a new device target for the controller 
-        light_device_target_t *backlight_controller = malloc(sizeof(light_device_target_t));
-        snprintf(backlight_controller->name, sizeof(backlight_controller->name), "%s", curr_entry->d_name);
-        
-        // Setup the function bindings
-        backlight_controller->set_value = impl_sysfs_set;
-        backlight_controller->get_value = impl_sysfs_get;
-        backlight_controller->get_max_value = impl_sysfs_getmax;
-        backlight_controller->custom_command = impl_sysfs_command;
-        
         // Setup the target data 
         impl_sysfs_data_t *dev_data = malloc(sizeof(impl_sysfs_data_t));
-        backlight_controller->device_target_data = dev_data;
         snprintf(dev_data->brightness, sizeof(dev_data->brightness), "/sys/class/backlight/%s/brightness", curr_entry->d_name);
         snprintf(dev_data->max_brightness, sizeof(dev_data->max_brightness), "/sys/class/backlight/%s/max_brightness", curr_entry->d_name);
         
+        // Create a new device target for the controller 
+        light_create_device_target(backlight_device, curr_entry->d_name, impl_sysfs_set, impl_sysfs_get, impl_sysfs_getmax, impl_sysfs_command, dev_data);
+        
         // Read the max brightness to get the best one
         uint64_t curr_value = 0;
         if(light_file_read_uint64(dev_data->max_brightness, &curr_value))
@@ -113,34 +86,24 @@ static bool _impl_sysfs_init_backlight(light_device_enumerator_t *enumerator)
             if(curr_value > best_value)
             {
                 best_value = curr_value;
-                snprintf(best_controller, sizeof(best_controller), "%s", backlight_controller->name);
+                snprintf(best_controller, sizeof(best_controller), "%s", curr_entry->d_name);
             }
         }
-        
-        // Add it to the device
-        light_add_device_target(backlight_device, backlight_controller);
     }
     
     closedir(backlight_dir);
     
-    // Create an auto controller 
-    light_device_target_t *auto_controller = malloc(sizeof(light_device_target_t));
-    snprintf(auto_controller->name, sizeof(auto_controller->name), "%s", "auto");
-    
-    // Setup the function bindings
-    auto_controller->set_value = impl_sysfs_set;
-    auto_controller->get_value = impl_sysfs_get;
-    auto_controller->get_max_value = impl_sysfs_getmax;
-    auto_controller->custom_command = impl_sysfs_command;
-    
-    // Setup the target data 
-    impl_sysfs_data_t *dev_data = malloc(sizeof(impl_sysfs_data_t));
-    auto_controller->device_target_data = dev_data;
-    snprintf(dev_data->brightness, sizeof(dev_data->brightness), "/sys/class/backlight/%s/brightness", best_controller);
-    snprintf(dev_data->max_brightness, sizeof(dev_data->max_brightness), "/sys/class/backlight/%s/max_brightness", best_controller);
-    
-    // Add it to the device
-    light_add_device_target(backlight_device, auto_controller);
+    // If we found at least one usable controller, create an auto target mapped to that controller
+    if(best_value > 0)
+    {
+        // Setup the target data 
+        impl_sysfs_data_t *dev_data = malloc(sizeof(impl_sysfs_data_t));
+        snprintf(dev_data->brightness, sizeof(dev_data->brightness), "/sys/class/backlight/%s/brightness", best_controller);
+        snprintf(dev_data->max_brightness, sizeof(dev_data->max_brightness), "/sys/class/backlight/%s/max_brightness", best_controller);
+        
+        // Create a new device target for the controller 
+        light_create_device_target(backlight_device, "auto", impl_sysfs_set, impl_sysfs_get, impl_sysfs_getmax, impl_sysfs_command, dev_data);
+    }
     
     return true;
 }
@@ -158,46 +121,6 @@ bool impl_sysfs_init(light_device_enumerator_t *enumerator)
 
 bool impl_sysfs_free(light_device_enumerator_t *enumerator)
 {
-    // Iterate through the devices in the enumerator
-    for(uint64_t d = 0; d < enumerator->num_devices; d++)
-    {
-        light_device_t *curr_device = enumerator->devices[d];
-        
-        // If the given device points to NULL, we can safely skip it
-        if(curr_device == NULL)
-        {
-            continue;
-        }
-        
-        for(uint64_t t = 0; t < curr_device->num_targets; t++)
-        {
-            light_device_target_t *curr_target = curr_device->targets[t];
-            
-            if(curr_target == NULL)
-            {
-                continue;
-            }
-            
-            if(curr_target->device_target_data != NULL)
-            {
-                free(curr_target->device_target_data);
-            }
-            
-            free(curr_target);
-        }
-        
-        // If the given device has any device_data, free it
-        if(curr_device->device_data != NULL)
-        {
-            free(curr_device->device_data);
-        }
-                
-        light_dispose_device(curr_device);    
-
-        // Free the device
-        free(curr_device);
-    }
-    
     return true;
 }
 

src/light.c

@@ -16,6 +16,59 @@
 
 /* Static helper functions for this file only, prefix with _ */
 
+
+static void _light_add_enumerator_device(light_device_enumerator_t *enumerator, light_device_t *new_device)
+{
+    // Create a new device array 
+    uint64_t new_num_devices = enumerator->num_devices + 1;
+    light_device_t **new_devices = malloc(new_num_devices * sizeof(light_device_t*));
+    
+    // Copy old device array to new one
+    for(uint64_t i = 0; i < enumerator->num_devices; i++)
+    {
+        new_devices[i] = enumerator->devices[i];
+    }
+    
+    // Set the new device
+    new_devices[enumerator->num_devices] = new_device;
+    
+    // Free the old devices array, if needed
+    if(enumerator->devices != NULL)
+    {
+        free(enumerator->devices);
+    }
+    
+    // Replace the devices array with the new one
+    enumerator->devices = new_devices;
+    enumerator->num_devices = new_num_devices;
+}
+
+static void _light_add_device_target(light_device_t *device, light_device_target_t *new_target)
+{
+    // Create a new targets array 
+    uint64_t new_num_targets = device->num_targets + 1;
+    light_device_target_t **new_targets = malloc(new_num_targets * sizeof(light_device_target_t*));
+    
+    // Copy old targets array to new one
+    for(uint64_t i = 0; i < device->num_targets; i++)
+    {
+        new_targets[i] = device->targets[i];
+    }
+    
+    // Set the new target
+    new_targets[device->num_targets] = new_target;
+    
+    // Free the old targets array, if needed
+    if(device->targets != NULL)
+    {
+        free(device->targets);
+    }
+    
+    // Replace the targets array with the new one
+    device->targets= new_targets;
+    device->num_targets = new_num_targets;
+}
+
 static void _light_get_target_path(light_context_t* ctx, char* output_path, size_t output_size)
 {
     snprintf(output_path, output_size,
@@ -39,7 +92,6 @@ static void _light_get_target_file(light_context_t* ctx, char* output_path, size
             );
 }
 
-
 static uint64_t _light_get_min_cap(light_context_t *ctx)
 {
     char target_path[NAME_MAX];
@@ -435,7 +487,7 @@ light_device_enumerator_t * light_create_enumerator(light_context_t *ctx, char c
     }
     
     // Allocate the new enumerator
-    new_enumerators[ctx->num_enumerators] = malloc(sizeof(light_device_enumerator_t*));
+    new_enumerators[ctx->num_enumerators] = malloc(sizeof(light_device_enumerator_t));
     light_device_enumerator_t *returner = new_enumerators[ctx->num_enumerators];
     
     returner->devices = NULL;
@@ -473,23 +525,13 @@ bool light_init_enumerators(light_context_t *ctx)
     return success;
 }
 
-void light_dispose_device(light_device_t *device)
-{
-    if(device->targets != NULL)
-    {
-        free(device->targets);
-        device->targets = NULL;
-    }
-    
-    device->num_targets = 0;
-}
-
 bool light_free_enumerators(light_context_t *ctx)
 {
     bool success = true;
     for(uint64_t i = 0; i < ctx->num_enumerators; i++)
     {
         light_device_enumerator_t * curr_enumerator = ctx->enumerators[i];
+        
         if(!curr_enumerator->free(curr_enumerator))
         {
             success = false;
@@ -497,6 +539,11 @@ bool light_free_enumerators(light_context_t *ctx)
         
         if(curr_enumerator->devices != NULL)
         {
+            for(uint64_t d = 0; d < curr_enumerator->num_devices; d++)
+            {
+                light_delete_device(curr_enumerator->devices[d]);
+            }
+            
             free(curr_enumerator->devices);
             curr_enumerator->devices = NULL;
         }
@@ -511,63 +558,6 @@ bool light_free_enumerators(light_context_t *ctx)
     return success;
 }
 
-void light_add_enumerator_device(light_device_enumerator_t *enumerator, light_device_t *new_device)
-{
-    // Create a new device array 
-    uint64_t new_num_devices = enumerator->num_devices + 1;
-    light_device_t **new_devices = malloc(new_num_devices * sizeof(light_device_t*));
-    
-    // Copy old device array to new one
-    for(uint64_t i = 0; i < enumerator->num_devices; i++)
-    {
-        new_devices[i] = enumerator->devices[i];
-    }
-    
-    // Set the new device
-    new_devices[enumerator->num_devices] = new_device;
-    
-    // Free the old devices array, if needed
-    if(enumerator->devices != NULL)
-    {
-        free(enumerator->devices);
-    }
-    
-    // Replace the devices array with the new one
-    enumerator->devices = new_devices;
-    enumerator->num_devices = new_num_devices;
-    
-    new_device->enumerator = enumerator;
-}
-
-void light_add_device_target(light_device_t *device, light_device_target_t *new_target)
-{
-    // Create a new targets array 
-    uint64_t new_num_targets = device->num_targets + 1;
-    light_device_target_t **new_targets = malloc(new_num_targets * sizeof(light_device_target_t*));
-    
-    // Copy old targets array to new one
-    for(uint64_t i = 0; i < device->num_targets; i++)
-    {
-        new_targets[i] = device->targets[i];
-    }
-    
-    // Set the new target
-    new_targets[device->num_targets] = new_target;
-    
-    // Free the old targets array, if needed
-    if(device->targets != NULL)
-    {
-        free(device->targets);
-    }
-    
-    // Replace the targets array with the new one
-    device->targets= new_targets;
-    device->num_targets = new_num_targets;
-    
-    new_target->device = device;
-}
-
-
 bool light_split_target_path(char const *in_path, light_target_path_t *out_path)
 {
     char const * begin = in_path;
@@ -966,6 +956,64 @@ bool light_cmd_restore_brightness(light_context_t *ctx)
     return true;
 }
 
+light_device_t *light_create_device(light_device_enumerator_t *enumerator, char const *name, void *device_data)
+{
+    light_device_t *new_device = malloc(sizeof(light_device_t));
+    new_device->enumerator = enumerator;
+    new_device->targets = NULL;
+    new_device->num_targets = 0;
+    new_device->device_data = device_data;
+    
+    snprintf(new_device->name, sizeof(new_device->name), "%s", name);
+    
+    _light_add_enumerator_device(enumerator, new_device);
+    
+    return new_device;
+}
 
+void light_delete_device(light_device_t *device)
+{
+    for(uint64_t i = 0; i < device->num_targets; i++)
+    {
+        light_delete_device_target(device->targets[i]);
+    }
+    
+    if(device->targets != NULL)
+    {
+        free(device->targets);
+    }
+    
+    if(device->device_data != NULL)
+    {
+        free(device->device_data);
+    }
+    
+    free(device);
+}
 
+light_device_target_t *light_create_device_target(light_device_t *device, char const *name, LFUNCVALSET setfunc, LFUNCVALGET getfunc, LFUNCMAXVALGET getmaxfunc, LFUNCCUSTOMCMD cmdfunc, void *target_data)
+{
+    light_device_target_t *new_target = malloc(sizeof(light_device_target_t));
+    new_target->device = device;
+    new_target->set_value = setfunc;
+    new_target->get_value = getfunc;
+    new_target->get_max_value = getmaxfunc;
+    new_target->custom_command = cmdfunc;
+    new_target->device_target_data = target_data;
+    
+    snprintf(new_target->name, sizeof(new_target->name), "%s", name);
+    
+    _light_add_device_target(device, new_target);
+    
+    return new_target;
+}
 
+void light_delete_device_target(light_device_target_t *device_target)
+{
+    if(device_target->device_target_data != NULL)
+    {
+        free(device_target->device_target_data);
+    }
+    
+    free(device_target);
+}

src/light.h

@@ -119,9 +119,17 @@ bool light_init_enumerators(light_context_t *ctx);
 /* Frees all the device enumerators (and its devices, targets) */
 bool light_free_enumerators(light_context_t *ctx);
 
-void light_add_enumerator_device(light_device_enumerator_t *enumerator, light_device_t *new_device);
+/* Use this to create a device. Will automatically be added to enumerator. */
+light_device_t *light_create_device(light_device_enumerator_t *enumerator, char const *name, void *device_data);
 
-void light_add_device_target(light_device_t *device, light_device_target_t *new_target);
+/* Use this to delete a device. */
+void light_delete_device(light_device_t *device);
+
+/* Use this to create a device target. Will automatically be added to device. */
+light_device_target_t *light_create_device_target(light_device_t *device, char const *name, LFUNCVALSET setfunc, LFUNCVALGET getfunc, LFUNCMAXVALGET getmaxfunc, LFUNCCUSTOMCMD cmdfunc, void *target_data);
+
+/* Use this to delete a device target. */
+void light_delete_device_target(light_device_target_t *device_target);
 
 typedef struct _light_target_path_t light_target_path_t;
 struct _light_target_path_t 
@@ -136,5 +144,3 @@ bool light_split_target_path(char const * in_path, light_target_path_t *out_path
 /* Returns the found device target, or null. Name should be enumerator/device/target */
 light_device_target_t* light_find_device_target(light_context_t *ctx, char const * name);
 
-/* Frees any runtime-allocated data inside a device. Call this before you free a device. */
-void light_dispose_device(light_device_t *device);