Skip to contentSkip to conversation

src/gprs.c: make sure that the context is properly released

PATCH In review Giacinto Cifelli 2 participants 6 messages Sep 20, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 src/gprs.c: make sure that the context is properly released

---
 src/gprs.c | 4 ++++
 1 file changed, 4 insertions(+)
src/gprs.c +4 -0
index 8f5d195..40f43e3 100644
@@ -1642,6 +1642,9 @@ static void release_active_contexts(struct ofono_gprs *gprs)
 
 		if (gc->driver->detach_shutdown != NULL)
 			gc->driver->detach_shutdown(gc, ctx->context.cid);
+
+		/* Make sure the context is properly cleared */
+		release_context(ctx);
 	}
 }
 
@@ -2241,6 +2244,7 @@ static DBusMessage *gprs_remove_context(DBusConnection *conn,
 	}
 
 	DBG("Unregistering context: %s", ctx->path);
+	release_context(ctx);
 	context_dbus_unregister(ctx);
 	gprs->contexts = g_slist_remove(gprs->contexts, ctx);
 
Signature · 1 line
1.9.1
#1 lore raw

Hi Giacinto,

Quoted 19 lines · Giacinto Cifelli
---
  src/gprs.c | 4 ++++
  1 file changed, 4 insertions(+)
src/gprs.c +3 -0
index 8f5d195..40f43e3 100644
@@ -1642,6 +1642,9 @@ static void release_active_contexts(struct ofono_gprs *gprs)
 
  		if (gc->driver->detach_shutdown != NULL)
  			gc->driver->detach_shutdown(gc, ctx->context.cid);
+
+		/* Make sure the context is properly cleared */
+		release_context(ctx);
  	}
  }
 

src/gprs.c static DBusMessage *gprs_remove_context(DBusConnection *conn, · line 2247

Quoted 4 lines
src/gprs.c +1 -0
@@ -2241,6 +2244,7 @@ static DBusMessage *gprs_remove_context(DBusConnection *conn,
  	}
 
  	DBG("Unregistering context: %s", ctx->path);
+	release_context(ctx);

If we get here then the context isn't active and doesn't need to be released. Active context release happens in gprs_deactivate_for_remove.
So I don't think this is needed?

src/gprs.c static DBusMessage *gprs_remove_context(DBusConnection *conn, · line 2250

Quoted 4 lines
src/gprs.c +0 -0
  	context_dbus_unregister(ctx);
  	gprs->contexts = g_slist_remove(gprs->contexts, ctx);
 

Regards,
-Denis

#2 lore raw

Hi Denis,

Quoted 24 lines · On Thu, 20 Sep 2018, 17:38 Denis Kenzior, <[email protected]> wrote:

Hi Giacinto,

src/gprs.c static void release_active_contexts(struct

Quoted 9 lines · Giacinto Cifelli
---
  src/gprs.c | 4 ++++
  1 file changed, 4 insertions(+)
src/gprs.c +0 -0
index 8f5d195..40f43e3 100644
@@ -1642,6 +1642,9 @@ static void release_active_contexts(struct

ofono_gprs *gprs)

src/gprs.c static DBusMessage

Quoted 10 lines
src/gprs.c +3 -0
              if (gc->driver->detach_shutdown != NULL)
                      gc->driver->detach_shutdown(gc, ctx->context.cid);
+
+             /* Make sure the context is properly cleared */
+             release_context(ctx);
      }
  }
 
@@ -2241,6 +2244,7 @@ static DBusMessage

*gprs_remove_context(DBusConnection *conn,

Quoted 9 lines

src/gprs.c static DBusMessage *gprs_remove_context(DBusConnection *conn, · line 2247

Quoted 4 lines
src/gprs.c +1 -0
      }
 
      DBG("Unregistering context: %s", ctx->path);
+     release_context(ctx);

If we get here then the context isn't active and doesn't need to be released. Active context release happens in gprs_deactivate_for_remove.
So I don't think this is needed?

I got this change from external branches.

I doubted it was needed anymore, but couldn't be sure that I had tested all cases.

thank you for checking it, and it can then be left out.

Quoted 8 lines

src/gprs.c static DBusMessage *gprs_remove_context(DBusConnection *conn, · line 2250

Quoted 4 lines
src/gprs.c +0 -0
      context_dbus_unregister(ctx);
      gprs->contexts = g_slist_remove(gprs->contexts, ctx);
 

Regards,
-Denis

Regards,
Giacinto

Quoted 1 line
#3 lore raw

Hi Giacinto,

Quoted 6 lines

I got this change from external branches.

I doubted it was needed anymore, but couldn't be sure that I had tested all cases.

thank you for checking it, and it can then be left out.

I can apply this patch without the chunk in question. Or do you want to resubmit?

Regards,
-Denis

#4 lore raw

Hi Denis,

Quoted 4 lines · Denis Kenzior

Hi Giacinto,

Quoted 2 lines

I got this change from external branches.

Quoted 8 lines
Quoted 4 lines

I doubted it was needed anymore, but couldn't be sure that I had tested all cases.

thank you for checking it, and it can then be left out.

I can apply this patch without the chunk in question. Or do you want to resubmit?

If you can apply directly it would be great, thanks.

Quoted 4 lines

Regards,
-Denis

Regards,
Giacinto

#5 lore raw

Hi Giacinto,

Quoted 2 lines

If you can apply directly it would be great, thanks.

Okay, applied with just the first chunk and a tweaked commit message.

Regards,
-Denis