Add some comments to the code
authorSteven Luo <steven+maemo@steven676.net>
Tue, 15 Dec 2009 06:37:53 +0000 (22:37 -0800)
committerSteven Luo <steven+maemo@steven676.net>
Tue, 15 Dec 2009 06:37:53 +0000 (22:37 -0800)
Restore some of the comments from the Python script, plus a few other
points that might make the code clearer.

dbus-server-bindings.c
launcher.c
main.c

index b29b1ab..939056b 100644 (file)
@@ -51,7 +51,10 @@ static void open_address(const char *uri) {
                /* Not much to do in this case ... */
                return;
 
+       printf("open_address '%s'\n", uri);
        if (uri[0] == '/') {
+               /* URI begins with a '/' -- assume it points to a local file
+                  and prefix with "file://" */
                new_uri_len = strlen("file://") + strlen(uri) + 1;
                if (!(new_uri = calloc(new_uri_len, sizeof(char))))
                        exit(1);
@@ -62,7 +65,6 @@ static void open_address(const char *uri) {
                   we need to clean up after ourselves */
                free(new_uri);
        } else {
-               printf("open_address '%s'\n", uri);
                launch_browser(&ctx, (char *)uri);
        }
 }
index e77e965..3ea9f80 100644 (file)
@@ -46,6 +46,12 @@ static void launch_tear(struct swb_context *ctx, char *uri) {
 
        printf("launch_tear with uri '%s'\n", uri);
 
+       /* We should be able to just call the D-Bus service to open Tear ...
+          but if Tear's not open, that cuases D-Bus to star Tear and then pass
+          it the OpenAddress call, which results in two browser windows.
+          Properly fixing this probably requires Tear to provide a D-Bus
+          method that opens an address in an existing window, but for now work
+          around by just invoking Tear with exec() if it's not running. */
        status = system("pidof tear > /dev/null");
        if (WIFEXITED(status) && !WEXITSTATUS(status)) {
                if (!tear_proxy)
@@ -78,12 +84,14 @@ void launch_microb(struct swb_context *ctx, char *uri) {
        if (!uri)
                uri = "new_window";
 
+       /* Launch browserd if it's not running */
        status = system("pidof /usr/sbin/browserd > /dev/null");
        if (WIFEXITED(status) && WEXITSTATUS(status)) {
                kill_browserd = 1;
                system("/usr/sbin/browserd -d");
        }
 
+       /* Release the osso_browser D-Bus name so that MicroB can take it */
        dbus_release_osso_browser_name(ctx);
 
        if ((pid = fork()) == -1) {
@@ -95,6 +103,9 @@ void launch_microb(struct swb_context *ctx, char *uri) {
                waitpid(pid, &status, 0);
        } else {
                /* Child process */
+               /* exec maemo-invoker directly instead of relying on the
+                  /usr/bin/browser symlink, since /usr/bin/browser may have
+                  been replaced with a shell script calling us via D-Bus */
                if (!strcmp(uri, "new_window")) {
                        execl("/usr/bin/maemo-invoker",
                              "browser", (char *)NULL);
@@ -104,6 +115,7 @@ void launch_microb(struct swb_context *ctx, char *uri) {
                }
        }
 
+       /* Kill off browserd if we started it */
        if (kill_browserd)
                system("kill `pidof /usr/sbin/browserd`");
 
@@ -125,7 +137,7 @@ static void launch_other_browser(struct swb_context *ctx, char *uri) {
                uri = "";
        urilen = strlen(uri);
        if (urilen > 0) {
-               /* Quote the URI */
+               /* Quote the URI to prevent the shell from interpreting it */
                /* urilen+3 = length of URI + 2x \' + \0 */
                if (!(quoted_uri = calloc(urilen+3, sizeof(char))))
                        exit(1);
@@ -187,6 +199,10 @@ static void launch_other_browser(struct swb_context *ctx, char *uri) {
        execl("/bin/sh", "/bin/sh", "-c", command, (char *)NULL);
 }
 
+/* Use launch_other_browser as the default browser launcher, with the string
+   passed in as the other_browser_cmd
+   Resulting other_browser_cmd is always safe to free(), even if a pointer
+   to a string constant is passed in */
 static void use_other_browser_cmd(struct swb_context *ctx, char *cmd) {
        size_t len = strlen(cmd);
 
@@ -221,6 +237,8 @@ void update_default_browser(struct swb_context *ctx, char *default_browser) {
        else if (!strcmp(default_browser, "microb"))
                ctx->default_browser_launcher = launch_microb;
        else if (!strcmp(default_browser, "fennec"))
+               /* Cheat and reuse launch_other_browser, since we don't appear
+                  to need to do anything special */
                use_other_browser_cmd(ctx, "fennec %s");
        else if (!strcmp(default_browser, "midori"))
                use_other_browser_cmd(ctx, "midori %s");
diff --git a/main.c b/main.c
index 66591c6..9adacbe 100644 (file)
--- a/main.c
+++ b/main.c
@@ -67,6 +67,7 @@ static void read_config(int signalnum) {
 
        set_config_defaults(&ctx);
 
+       /* Put together the path to the config file */
        if (!(homedir = getenv("HOME")))
                homedir = DEFAULT_HOMEDIR;
        len = strlen(homedir) + strlen(CONFIGFILE_LOC) + 1;
@@ -74,6 +75,7 @@ static void read_config(int signalnum) {
                goto out_noopen;
        snprintf(configfile, len, "%s%s", homedir, CONFIGFILE_LOC);
 
+       /* Try to open the config file */
        if (!(fp = fopen(configfile, "r"))) {
                /* Try the legacy config file location before giving up
                   XXX we assume here that CONFIGFILE_LOC_OLD is shorter
@@ -174,16 +176,19 @@ int main() {
        read_config(0);
 
        if (ctx.continuous_mode) {
+               /* Install signal handlers */
                struct sigaction act;
                act.sa_flags = SA_RESTART;
                sigemptyset(&(act.sa_mask));
 
+               /* SIGCHLD -- clean up after zombies */
                act.sa_handler = waitforzombies;
                if (sigaction(SIGCHLD, &act, NULL) == -1) {
                        printf("Installing signal handler failed\n");
                        return 1;
                }
 
+               /* SIGHUP -- reread config file */
                act.sa_handler = read_config;
                if (sigaction(SIGHUP, &act, NULL) == -1) {
                        printf("Installing signal handler failed\n");
@@ -196,6 +201,7 @@ int main() {
        dbus_g_object_type_install_info(OSSO_BROWSER_TYPE,
                        &dbus_glib_osso_browser_object_info);
 
+       /* Get a connection to the D-Bus session bus */
        ctx.session_bus = dbus_g_bus_get(DBUS_BUS_SESSION, &error);
        if (!ctx.session_bus) {
                printf("Couldn't get a D-Bus bus connection\n");
@@ -211,6 +217,7 @@ int main() {
 
        dbus_request_osso_browser_name(&ctx);
 
+       /* Register ourselves to handle the osso_browser D-Bus methods */
        obj_osso_browser = g_object_new(OSSO_BROWSER_TYPE, NULL);
        obj_osso_browser_req = g_object_new(OSSO_BROWSER_TYPE, NULL);
        dbus_g_connection_register_g_object(ctx.session_bus,