On 30/09/2019 22.59, Denis Kenzior wrote:
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);

I chose to stick with the unconditional reference. Your suggestion can cause the mux object to be freed when unref'ed after the loop, but the function accesses it again later in

        if (mux->buf_used == sizeof(mux->buf))

The unconditional reference makes the flow nicer, IMO.

// Martin
_______________________________________________
ofono mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to