Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
* appsymc code fixes (wrong assumptions)
* make udev more robust
* cleaner module checking

Signed-off-by: Philippe De Swert <phdeswer@lumi.maa>
  • Loading branch information
Philippe De Swert committed Apr 20, 2011
1 parent 2554770 commit d69981b
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 23 deletions.
25 changes: 17 additions & 8 deletions src/usb_moded-appsync.c
Expand Up @@ -52,7 +52,7 @@ GList *readlist(void)
if(list_item)
applist = g_list_append(applist, list_item);
}

g_dir_close(confdir);
}
else
log_debug("confdir open failed.\n");
Expand Down Expand Up @@ -91,7 +91,7 @@ static struct list_elem *read_file(const gchar *filename)
log_debug("Appname = %s\n", name_char);
}
}
if(!strcmp(*keys, APP_INFO_LAUNCH_KEY))
else if(!strcmp(*keys, APP_INFO_LAUNCH_KEY))
{
launch_char = g_key_file_get_string(settingsfile, APP_INFO_ENTRY, *keys, NULL);
if(launch_char)
Expand All @@ -115,13 +115,17 @@ static struct list_elem *read_file(const gchar *filename)

int activate_sync(GList *list)
{
GList *list_iter;

list_iter = list;
/* set list to inactive */
do
{
struct list_elem *data = list->data;
struct list_elem *data = list_iter->data;
data->active = 0;
list_iter = g_list_next(list_iter);
}
while(g_list_next(list) != NULL);
while(list_iter != NULL);

/* add dbus filter. Use session bus for ready method call? */
if(!usb_moded_app_sync_init(list))
Expand All @@ -132,13 +136,15 @@ int activate_sync(GList *list)
}

/* go through list and launch apps */
list_iter = list;
do
{
struct list_elem *data = list->data;
struct list_elem *data = list_iter->data;
log_debug("launching app %s\n", data->launch);
usb_moded_dbus_app_launch(data->launch);
list_iter = g_list_next(list_iter);
}
while(g_list_next(list) != NULL);
while(list_iter != NULL);

/* start timer */
log_debug("Starting timer\n");
Expand All @@ -152,14 +158,16 @@ int mark_active(GList *list, const gchar *name)
int ret = 0;
static int list_length=0;
int counter=0;
GList *list_iter;

log_debug("app %s notified it is ready\n", name);
if(list_length == 0)
list_length = g_list_length(list);

list_iter = list;
do
{
struct list_elem *data = list->data;
struct list_elem *data = list_iter->data;
if(!strcmp(data->name, name))
{
if(!data->active)
Expand All @@ -181,8 +189,9 @@ int mark_active(GList *list, const gchar *name)
ret = 1;
}
}
list_iter = g_list_next(list_iter);
}
while(g_list_next(list) != NULL);
while(list_iter != NULL);

return(ret);
}
Expand Down
49 changes: 38 additions & 11 deletions src/usb_moded-modules.c
Expand Up @@ -78,17 +78,36 @@ int usb_moded_unload_module(const char *module)
*/
const char * usb_moded_find_module(void)
{

if(system("lsmod | grep g_nokia") == 0)
return(MODULE_NETWORK);

if(system("lsmod | grep g_file_storage") == 0)
return(MODULE_MASS_STORAGE);

if(system("lsmod | grep g_ether") == 0)
return(MODULE_WINDOWS_NET);

return(NULL);
FILE *stream = 0;
const char *result = 0;

if( (stream = popen("lsmod", "r")) )
{
char *text = 0;
size_t size = 0;

while( getline(&text, &size, stream) >= 0 )
{
if( strstr(text, "g_nokia") )
{
result = MODULE_NETWORK;
break;
}
if( strstr(text, "g_file_storage") )
{
result = MODULE_MASS_STORAGE;
break;
}
if( strstr(text, "g_ether") )
{
result = MODULE_WINDOWS_NET;
break;
}
}
pclose(stream);
}

return result;
}

/** clean up for modules when usb gets disconnected
Expand All @@ -111,6 +130,7 @@ int usb_moded_module_cleanup(const char *module)
*/

success = usb_moded_unload_module(module);
// SP: variable 'success' holds error value?
while(success)
{
/* module did not get unloaded. We will wait a bit and try again */
Expand All @@ -124,6 +144,7 @@ int usb_moded_module_cleanup(const char *module)
retry++;
if(retry == 2)
break;
// SP: up to 2 second sleep -> worth a warning log?
}
if(!strcmp(module, MODULE_NETWORK))
{
Expand All @@ -142,12 +163,18 @@ int usb_moded_module_cleanup(const char *module)
system("for i in `lsof -t /dev/ttyGS*`; do kill -s SIGTERM $i ; done");
system("for i in `lsof -t /dev/gc*`; do kill -s SIGTERM $i ; done");
system("for i in `lsof -t /dev/mtp*`; do kill -s SIGTERM $i ; done");
// SP: three passes and for loops in sh?
// SP: system("kill -s SIGTERM $(lsof -t /dev/ttyGS* /dev/gc* /dev/mtp*") ?
// SP: or popen + kill loop?
/* try to unload again and give up if it did not work yet */
success = usb_moded_unload_module(module);
if(success && retry < 10)
{
retry++;
// SP: NOTE: we have a root process in busyloop sending kill signals to
// user processes? should we give them a chance to react?
goto kill;
// SP: IMHO backwards goto is bad - a loop perhaps?
}
if(success && retry == 10)
{
Expand Down
24 changes: 20 additions & 4 deletions src/usb_moded-udev.c
Expand Up @@ -29,13 +29,14 @@ gboolean hwal_init(void)
{
const gchar *udev_path = NULL;
struct udev_device *dev;
int ret = 0;

/* Create the udev object */
udev = udev_new();
if (!udev)
{
log_err("Can't create udev\n");
return 0;
return FALSE;
}

udev_path = find_udev_path();
Expand All @@ -46,28 +47,38 @@ gboolean hwal_init(void)
if (!dev)
{
log_err("Unable to find $power_supply device.");
return 0;
/* communicate failure, mainloop will exit and call appropriate clean-up */
return FALSE;
}
mon = udev_monitor_new_from_netlink (udev, "udev");
if (!mon)
{
log_err("Unable to monitor the 'present' value\n");
return 0;
/* communicate failure, mainloop will exit and call appropriate clean-up */
return FALSE;
}
udev_monitor_filter_add_match_subsystem_devtype(mon, "power_supply", NULL);
// SP: this can fail - in theory at least

udev_monitor_enable_receiving (mon);
// SP: this can fail - in theory at least

/* check if we are already connected */
udev_parse(dev);

iochannel = g_io_channel_unix_new(udev_monitor_get_fd(mon));
// SP: this can fail - in theory at least

/* default is UTF-8, set it to binary */
g_io_channel_set_encoding(iochannel, NULL, NULL);
/* set it to unbuffered, since we will be bypassing the GIOChannel for reads */
g_io_channel_set_buffered(iochannel, FALSE);
// SP: do these matter, we're not using g_io for reading?

watch_id = g_io_add_watch(iochannel, G_IO_IN, monitor_udev, NULL);
// SP: this can fail - in theory at least

/* everything went well */
return TRUE;
}

Expand All @@ -78,7 +89,6 @@ static gboolean monitor_udev(GIOChannel *iochannel G_GNUC_UNUSED, GIOCondition c

if(cond & G_IO_IN)
{

/* This normally blocks but G_IO_IN indicates that we can read */
dev = udev_monitor_receive_device (mon);
if (dev)
Expand All @@ -88,7 +98,11 @@ static gboolean monitor_udev(GIOChannel *iochannel G_GNUC_UNUSED, GIOCondition c
udev_parse(dev);
}
}
/* if we get something else something bad happened stop watching to avoid busylooping */
else
exit(1);
}

/* keep watching */
return TRUE;
}
Expand All @@ -114,6 +128,7 @@ static void udev_parse(struct udev_device *dev)
{
log_err("No usable power supply indicator\n");
exit(1);
// SP: exit? really?
}
if(!strcmp(tmp, "1"))
{
Expand Down Expand Up @@ -141,4 +156,5 @@ static void udev_parse(struct udev_device *dev)
}
END:
udev_device_unref(dev);
// SP: IMHO: either move the unref to caller or rename the function
}

0 comments on commit d69981b

Please sign in to comment.