Hi Martin,
@@ -227,12 +227,15 @@ static void dispatch_sources(GAtMuxChannel *channel,
GIOCondition condition)
static gboolean received_data(GIOChannel *channel, GIOCondition cond,
gpointer data)
{
- GAtMux *mux = data;
+ GAtMux *mux = g_at_mux_ref(data);
So you're taking a ref here unconditionally, yet...
int i;
GIOStatus status;
gsize bytes_read;
if (cond & G_IO_NVAL)
+ goto out_done;
+
+ if (mux->shutdown)
return FALSE;
You don't return it here?
debug(mux, "received data");
@@ -270,15 +273,22 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
}
if (cond & (G_IO_HUP | G_IO_ERR))
- return FALSE;
+ goto out_done;
if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
- return FALSE;
+ goto out_done;
if (mux->buf_used == sizeof(mux->buf))
- return FALSE;
+ goto out_done;
+
+ g_at_mux_unref(mux);
Why not do something like:
g_at_mux_ref(mux);
for (i = 1; i <= MAX_CHANNELS && !mux->shutdown; i++) {
int offset = i / 8;
int bit = i % 8;
if (!(mux->newdata[offset] & (1 << bit)))
continue;
dispatch_sources(mux->dlcs[i-1], G_IO_IN);
}
g_at_mux_unref(mux);
return TRUE;
+
+out_done:
+ g_at_mux_unref(mux);
+
+ return FALSE;
}
static void write_watcher_destroy_notify(gpointer user_data)
@@ -286,6 +296,9 @@ static void write_watcher_destroy_notify(gpointer user_data)
GAtMux *mux = user_data;
mux->write_watch = 0;
+
+ if (mux->read_watch <= 0 && mux->write_watch <= 0)
+ g_free(mux);
}
static gboolean can_write_data(GIOChannel *chan, GIOCondition cond,
@@ -297,6 +310,9 @@ static gboolean can_write_data(GIOChannel *chan,
GIOCondition cond,
if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR))
return FALSE;
+ if (mux->shutdown)
+ return FALSE;
+
debug(mux, "can write data");
for (dlc = 0; dlc < MAX_CHANNELS; dlc += 1) {
@@ -607,6 +623,7 @@ GAtMux *g_at_mux_new(GIOChannel *channel, const
GAtMuxDriver *driver)
if (mux == NULL)
return NULL;
+
mux->ref_count = 1;
mux->driver = driver;
mux->shutdown = TRUE;
@@ -642,7 +659,8 @@ void g_at_mux_unref(GAtMux *mux)
if (mux->driver->remove)
mux->driver->remove(mux);
- g_free(mux);
+ if (mux->read_watch <= 0 && mux->write_watch <= 0)
+ g_free(mux);
This looks dangerous to me. You're going to cause a crash like this.
You really want to call the disconnect function instead. But I suspect
this is a different patch entirely.
}
}
@@ -651,6 +669,9 @@ static void read_watcher_destroy_notify(gpointer user_data)
GAtMux *mux = user_data;
mux->read_watch = 0;
+
+ if (mux->read_watch <= 0 && mux->write_watch <= 0)
+ g_free(mux);
Same here
}
gboolean g_at_mux_start(GAtMux *mux)
@@ -695,6 +716,7 @@ gboolean g_at_mux_shutdown(GAtMux *mux)
continue;
channel_close((GIOChannel *) mux->dlcs[i], NULL);
+ mux->dlcs[i] = NULL;
This can probably be a separate cleanup patch.
}
if (mux->driver->shutdown)
Regards,
-Denis
_______________________________________________
ofono mailing list -- [email protected]
To unsubscribe send an email to [email protected]
