Skip to content

Commit

Permalink
[dyn-config] Fix miscellaneous issues
Browse files Browse the repository at this point in the history 
String fields in modedata_t structures are filled in via functions that
return 'gchar *' pointers - adjust structure data types and release
function accordingly.

The modelist_free() function uses g_list_foreach() iterator and a
callback function that is not appropriate for the action, and then uses
a cast to hide any compiler warnings - get rid of the cast by using
g_list_free_full() iterator and a suitable callback function.

The modelist_load() function looks complex and indiscriminately loads
all files from dynamic mode configuration directory. If there are for
example editor backup files present, this can lead to hard to detect
issues - simplify the logic and use glob pattern expansion to derive
list of files to load.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
  • Loading branch information
@spiiroin
spiiroin committed Apr 9, 2019
1 parent 390ccad commit 1c90322
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 125 deletions.
199 changes: 98 additions & 101 deletions src/usb_moded-dyn-config.c
View file
Expand Up @@ -31,6 +31,7 @@
#include "usb_moded-log.h"

#include <stdlib.h>
#include <glob.h>

/* ========================================================================= *
* Prototypes
Expand All @@ -40,7 +41,8 @@
* MODEDATA
* ------------------------------------------------------------------------- */

void modedata_free (modedata_t *list_item);
static void modedata_free_cb(gpointer self);
void modedata_free (modedata_t *self);
static gint modedata_sort_cb(gconstpointer a, gconstpointer b);
static modedata_t *modedata_load (const gchar *filename);

Expand All @@ -49,42 +51,52 @@ static modedata_t *modedata_load (const gchar *filename);
* ------------------------------------------------------------------------- */

void modelist_free(GList *modelist);
GList *modelist_load(int diag);
GList *modelist_load(bool diag);

/* ========================================================================= *
* MODEDATA
* ========================================================================= */

/** Type agnostice relase modedata_t object callback
*
* @param self Object pointer, or NULL
*/
static void
modedata_free_cb(gpointer self)
{
modedata_free(self);
}

/** Relase modedata_t object
*
* @param list_item Object pointer, or NULL
* @param self Object pointer, or NULL
*/
void
modedata_free(modedata_t *list_item)
modedata_free(modedata_t *self)
{
LOG_REGISTER_CONTEXT;

if( list_item ) {
free(list_item->mode_name);
free(list_item->mode_module);
free(list_item->network_interface);
free(list_item->sysfs_path);
free(list_item->sysfs_value);
free(list_item->sysfs_reset_value);
free(list_item->android_extra_sysfs_path);
free(list_item->android_extra_sysfs_value);
free(list_item->android_extra_sysfs_path2);
free(list_item->android_extra_sysfs_value2);
free(list_item->android_extra_sysfs_path3);
free(list_item->android_extra_sysfs_value3);
free(list_item->android_extra_sysfs_path4);
free(list_item->android_extra_sysfs_value4);
free(list_item->idProduct);
free(list_item->idVendorOverride);
if( self ) {
g_free(self->mode_name);
g_free(self->mode_module);
g_free(self->network_interface);
g_free(self->sysfs_path);
g_free(self->sysfs_value);
g_free(self->sysfs_reset_value);
g_free(self->android_extra_sysfs_path);
g_free(self->android_extra_sysfs_value);
g_free(self->android_extra_sysfs_path2);
g_free(self->android_extra_sysfs_value2);
g_free(self->android_extra_sysfs_path3);
g_free(self->android_extra_sysfs_value3);
g_free(self->android_extra_sysfs_path4);
g_free(self->android_extra_sysfs_value4);
g_free(self->idProduct);
g_free(self->idVendorOverride);
#ifdef CONNMAN
free(list_item->connman_tethering);
g_free(self->connman_tethering);
#endif
free(list_item);
free(self);
}
}

Expand Down Expand Up @@ -119,68 +131,69 @@ modedata_load(const gchar *filename)
{
LOG_REGISTER_CONTEXT;

bool success = false;
GKeyFile *settingsfile = g_key_file_new();
modedata_t *list_item = NULL;
modedata_t *self = NULL;
bool success = false;
GKeyFile *settingsfile = g_key_file_new();

if( !g_key_file_load_from_file(settingsfile, filename, G_KEY_FILE_NONE, NULL) ) {
log_err("%s: can't read mode configuration file", filename);
goto EXIT;
}

list_item = calloc(1, sizeof *list_item);
if( !(self = calloc(1, sizeof *self)) )
goto EXIT;

// [MODE_ENTRY = "mode"]
list_item->mode_name = g_key_file_get_string(settingsfile, MODE_ENTRY, MODE_NAME_KEY, NULL);
list_item->mode_module = g_key_file_get_string(settingsfile, MODE_ENTRY, MODE_MODULE_KEY, NULL);
self->mode_name = g_key_file_get_string(settingsfile, MODE_ENTRY, MODE_NAME_KEY, NULL);
self->mode_module = g_key_file_get_string(settingsfile, MODE_ENTRY, MODE_MODULE_KEY, NULL);

log_debug("Dynamic mode name = %s\n", list_item->mode_name);
log_debug("Dynamic mode module = %s\n", list_item->mode_module);
log_debug("Dynamic mode name = %s\n", self->mode_name);
log_debug("Dynamic mode module = %s\n", self->mode_module);

list_item->appsync = g_key_file_get_integer(settingsfile, MODE_ENTRY, MODE_NEEDS_APPSYNC_KEY, NULL);
list_item->mass_storage = g_key_file_get_integer(settingsfile, MODE_ENTRY, MODE_MASS_STORAGE_KEY, NULL);
list_item->network = g_key_file_get_integer(settingsfile, MODE_ENTRY, MODE_NETWORK_KEY, NULL);
list_item->network_interface = g_key_file_get_string(settingsfile, MODE_ENTRY, MODE_NETWORK_INTERFACE_KEY, NULL);
self->appsync = g_key_file_get_integer(settingsfile, MODE_ENTRY, MODE_NEEDS_APPSYNC_KEY, NULL);
self->mass_storage = g_key_file_get_integer(settingsfile, MODE_ENTRY, MODE_MASS_STORAGE_KEY, NULL);
self->network = g_key_file_get_integer(settingsfile, MODE_ENTRY, MODE_NETWORK_KEY, NULL);
self->network_interface = g_key_file_get_string(settingsfile, MODE_ENTRY, MODE_NETWORK_INTERFACE_KEY, NULL);

// [MODE_OPTIONS_ENTRY = "options"]
list_item->sysfs_path = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_SYSFS_PATH, NULL);
list_item->sysfs_value = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_SYSFS_VALUE, NULL);
list_item->sysfs_reset_value = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_SYSFS_RESET_VALUE, NULL);

list_item->android_extra_sysfs_path = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH, NULL);
list_item->android_extra_sysfs_path2 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH2, NULL);
list_item->android_extra_sysfs_path3 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH3, NULL);
list_item->android_extra_sysfs_path4 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH4, NULL);
list_item->android_extra_sysfs_value = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE, NULL);
list_item->android_extra_sysfs_value2 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE2, NULL);
list_item->android_extra_sysfs_value3 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE3, NULL);
list_item->android_extra_sysfs_value4 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE4, NULL);

list_item->idProduct = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_IDPRODUCT, NULL);
list_item->idVendorOverride = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_IDVENDOROVERRIDE, NULL);
list_item->nat = g_key_file_get_integer(settingsfile, MODE_OPTIONS_ENTRY, MODE_HAS_NAT, NULL);
list_item->dhcp_server = g_key_file_get_integer(settingsfile, MODE_OPTIONS_ENTRY, MODE_HAS_DHCP_SERVER, NULL);
self->sysfs_path = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_SYSFS_PATH, NULL);
self->sysfs_value = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_SYSFS_VALUE, NULL);
self->sysfs_reset_value = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_SYSFS_RESET_VALUE, NULL);

self->android_extra_sysfs_path = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH, NULL);
self->android_extra_sysfs_path2 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH2, NULL);
self->android_extra_sysfs_path3 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH3, NULL);
self->android_extra_sysfs_path4 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_PATH4, NULL);
self->android_extra_sysfs_value = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE, NULL);
self->android_extra_sysfs_value2 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE2, NULL);
self->android_extra_sysfs_value3 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE3, NULL);
self->android_extra_sysfs_value4 = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_ANDROID_EXTRA_SYSFS_VALUE4, NULL);

self->idProduct = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_IDPRODUCT, NULL);
self->idVendorOverride = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_IDVENDOROVERRIDE, NULL);
self->nat = g_key_file_get_integer(settingsfile, MODE_OPTIONS_ENTRY, MODE_HAS_NAT, NULL);
self->dhcp_server = g_key_file_get_integer(settingsfile, MODE_OPTIONS_ENTRY, MODE_HAS_DHCP_SERVER, NULL);
#ifdef CONNMAN
list_item->connman_tethering = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_CONNMAN_TETHERING, NULL);
self->connman_tethering = g_key_file_get_string(settingsfile, MODE_OPTIONS_ENTRY, MODE_CONNMAN_TETHERING, NULL);
#endif

//log_debug("Dynamic mode sysfs path = %s\n", list_item->sysfs_path);
//log_debug("Dynamic mode sysfs value = %s\n", list_item->sysfs_value);
//log_debug("Android extra mode sysfs path2 = %s\n", list_item->android_extra_sysfs_path2);
//log_debug("Android extra value2 = %s\n", list_item->android_extra_sysfs_value2);
//log_debug("Dynamic mode sysfs path = %s\n", self->sysfs_path);
//log_debug("Dynamic mode sysfs value = %s\n", self->sysfs_value);
//log_debug("Android extra mode sysfs path2 = %s\n", self->android_extra_sysfs_path2);
//log_debug("Android extra value2 = %s\n", self->android_extra_sysfs_value2);

if( list_item->mode_name == NULL || list_item->mode_module == NULL ) {
if( self->mode_name == NULL || self->mode_module == NULL ) {
log_err("%s: mode_name or mode_module not defined", filename);
goto EXIT;
}

if( list_item->network && list_item->network_interface == NULL) {
if( self->network && self->network_interface == NULL) {
log_err("%s: network not fully defined", filename);
goto EXIT;
}

if( (list_item->sysfs_path && !list_item->sysfs_value) ||
(list_item->sysfs_reset_value && !list_item->sysfs_path) ) {
if( (self->sysfs_path && !self->sysfs_value) ||
(self->sysfs_reset_value && !self->sysfs_path) ) {
/* In theory all of this is optional.
*
* In most cases 'sysfs_value' holds a list of functions to enable,
Expand All @@ -202,9 +215,9 @@ modedata_load(const gchar *filename)
g_key_file_free(settingsfile);

if( !success )
modedata_free(list_item), list_item = 0;
modedata_free(self), self = 0;

return list_item;
return self;
}

/* ========================================================================= *
Expand All @@ -220,55 +233,39 @@ modelist_free(GList *modelist)
{
LOG_REGISTER_CONTEXT;

if(modelist)
{
g_list_foreach(modelist, (GFunc) modedata_free, NULL);
g_list_free(modelist);
modelist = 0;
}
g_list_free_full(modelist, modedata_free_cb);
}

/** Load mode data files from configuration directory
*
* @param diag true to load diagnostic modes, false for normal modes
* @param diag true to load diagnostic modes, or
* false for normal modes
*
* @return List of mode data objects, or NULL
*/
GList *
modelist_load(int diag)
modelist_load(bool diag)
{
LOG_REGISTER_CONTEXT;

GDir *confdir;
GList *modelist = NULL;
const gchar *dirname;
modedata_t *list_item;
gchar *full_filename = NULL;

if(diag)
confdir = g_dir_open(DIAG_DIR_PATH, 0, NULL);
else
confdir = g_dir_open(MODE_DIR_PATH, 0, NULL);
if(confdir)
{
while((dirname = g_dir_read_name(confdir)) != NULL)
{
log_debug("Read file %s\n", dirname);
if(diag)
full_filename = g_strconcat(DIAG_DIR_PATH, "/", dirname, NULL);
else
full_filename = g_strconcat(MODE_DIR_PATH, "/", dirname, NULL);
list_item = modedata_load(full_filename);
/* free full_filename immediately as we do not use it anymore */
free(full_filename);
if(list_item)
modelist = g_list_append(modelist, list_item);
}
g_dir_close(confdir);
GList *modelist = 0;
const char *dirpath = diag ? DIAG_DIR_PATH : MODE_DIR_PATH;
gchar *pattern = g_strdup_printf("%s/*.ini", dirpath);
glob_t gb = {};

if( glob(pattern, 0, 0, &gb) != 0 )
log_debug("no mode configuration ini-files found");

for( size_t i = 0; i < gb.gl_pathc; ++i ) {
const char *filepath = gb.gl_pathv[i];
log_debug("Read file %s\n", filepath);
modedata_t *list_item = modedata_load(filepath);
if(list_item)
modelist = g_list_append(modelist, list_item);
}
else
log_debug("Mode confdir open failed or file is incomplete/invalid.\n");

modelist = g_list_sort (modelist, modedata_sort_cb);
return modelist;
globfree(&gb);
g_free(pattern);

return g_list_sort(modelist, modedata_sort_cb);
}
49 changes: 25 additions & 24 deletions src/usb_moded-dyn-config.h
View file
Expand Up @@ -30,6 +30,7 @@
#ifndef USB_MODED_DYN_CONFIG_H_
# define USB_MODED_DYN_CONFIG_H_

# include <stdbool.h>
# include <glib.h>

/* ========================================================================= *
Expand Down Expand Up @@ -95,29 +96,29 @@
*/
typedef struct modedata_t
{
char *mode_name; /**< Mode name */
char *mode_module; /**< Needed module for given mode */
int appsync; /**< Requires appsync or not */
int network; /**< Bring up network or not */
int mass_storage; /**< Use mass-storage functions */
char *network_interface; /**< Which network interface to bring up if network needs to be enabled */
char *sysfs_path; /**< Path to set sysfs options */
char *sysfs_value; /**< Option name/value to write to sysfs */
char *sysfs_reset_value; /**< Value to reset the the sysfs to default */
char *android_extra_sysfs_path; /**< Path for static value that never changes that needs to be set by sysfs :( */
char *android_extra_sysfs_value; /**< Static value that never changes that needs to be set by sysfs :( */
char *android_extra_sysfs_path2; /**< Path for static value that never changes that needs to be set by sysfs :( */
char *android_extra_sysfs_value2; /**< Static value that never changes that needs to be set by sysfs :( */
char *android_extra_sysfs_path3; /**< Path for static value that never changes that needs to be set by sysfs :( */
char *android_extra_sysfs_value3; /**< Static value that never changes that needs to be set by sysfs :( */
char *android_extra_sysfs_path4; /**< Path for static value that never changes that needs to be set by sysfs :( */
char *android_extra_sysfs_value4; /**< Static value that never changes that needs to be set by sysfs :( */
char *idProduct; /**< Product id to assign to a specific profile */
char *idVendorOverride; /**< Temporary vendor override for special modes used by odms in testing/manufacturing */
int nat; /**< If NAT should be set up in this mode or not */
int dhcp_server; /**< if a DHCP server needs to be configured and started or not */
gchar *mode_name; /**< Mode name */
gchar *mode_module; /**< Needed module for given mode */
int appsync; /**< Requires appsync or not */
int network; /**< Bring up network or not */
int mass_storage; /**< Use mass-storage functions */
gchar *network_interface; /**< Which network interface to bring up if network needs to be enabled */
gchar *sysfs_path; /**< Path to set sysfs options */
gchar *sysfs_value; /**< Option name/value to write to sysfs */
gchar *sysfs_reset_value; /**< Value to reset the the sysfs to default */
gchar *android_extra_sysfs_path; /**< Path for static value that never changes that needs to be set by sysfs :( */
gchar *android_extra_sysfs_value; /**< Static value that never changes that needs to be set by sysfs :( */
gchar *android_extra_sysfs_path2; /**< Path for static value that never changes that needs to be set by sysfs :( */
gchar *android_extra_sysfs_value2; /**< Static value that never changes that needs to be set by sysfs :( */
gchar *android_extra_sysfs_path3; /**< Path for static value that never changes that needs to be set by sysfs :( */
gchar *android_extra_sysfs_value3; /**< Static value that never changes that needs to be set by sysfs :( */
gchar *android_extra_sysfs_path4; /**< Path for static value that never changes that needs to be set by sysfs :( */
gchar *android_extra_sysfs_value4; /**< Static value that never changes that needs to be set by sysfs :( */
gchar *idProduct; /**< Product id to assign to a specific profile */
gchar *idVendorOverride; /**< Temporary vendor override for special modes used by odms in testing/manufacturing */
int nat; /**< If NAT should be set up in this mode or not */
int dhcp_server; /**< if a DHCP server needs to be configured and started or not */
# ifdef CONNMAN
char* connman_tethering; /**< Connman's tethering technology path */
gchar *connman_tethering; /**< Connman's tethering technology path */
# endif
} modedata_t;

Expand All @@ -129,13 +130,13 @@ typedef struct modedata_t
* MODEDATA
* ------------------------------------------------------------------------- */

void modedata_free(modedata_t *list_item);
void modedata_free(modedata_t *self);

/* ------------------------------------------------------------------------- *
* MODELIST
* ------------------------------------------------------------------------- */

void modelist_free(GList *modelist);
GList *modelist_load(int diag);
GList *modelist_load(bool diag);

#endif /* USB_MODED_DYN_CONFIG_H_ */

0 comments on commit 1c90322

Please sign in to comment.