ofono.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* Documentation on when to free structures in/after callbacks
@ 2021-07-28 16:32 ajlennon
  2021-07-28 17:31 ` Denis Kenzior
  0 siblings, 1 reply; 7+ messages in thread
From: ajlennon @ 2021-07-28 16:32 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

Hi all,

I'm trying to write a driver for Quectel modems as I need some specific bits and bobs - mainly signal strengths for the serving cell and neighbourhood cells

I can't quite get my head around when I am supposed to provide free() functions to to calls and when to free in callbacks on success or error conditions.

Is there a bit of a document somewhere somebody could direct me to that explains this?

Or maybe I am overthinking and somebody could just explain the logic behind it?

Thanks!

Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Documentation on when to free structures in/after callbacks
  2021-07-28 16:32 Documentation on when to free structures in/after callbacks ajlennon
@ 2021-07-28 17:31 ` Denis Kenzior
  2021-07-30 13:21   ` ajlennon
  0 siblings, 1 reply; 7+ messages in thread
From: Denis Kenzior @ 2021-07-28 17:31 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

Hi Alex,

On 7/28/21 11:32 AM, ajlennon(a)dynamicdevices.co.uk wrote:
> Hi all,
> 
> I'm trying to write a driver for Quectel modems as I need some specific bits and bobs - mainly signal strengths for the serving cell and neighbourhood cells
> 
> I can't quite get my head around when I am supposed to provide free() functions to to calls and when to free in callbacks on success or error conditions.

I'm not sure what you mean?  What exactly do you think needs freeing?

Everything from the driver going to the core are const * or passed by value. 
There should be no instances of the core taking ownership of any memory from the 
driver.  To put another way, if the driver mallocs something, then the driver is 
responsible for freeing it.

Similarly, everything being passed in from the core to the driver is a const * 
or passed by value.

Fundamentally, there should never be any resource ownership change between 
driver and core...

> 
> Is there a bit of a document somewhere somebody could direct me to that explains this?
> 
> Or maybe I am overthinking and somebody could just explain the logic behind it?
> 
> Thanks!
> 
> Alex
> _______________________________________________
> ofono mailing list -- ofono(a)ofono.org
> To unsubscribe send an email to ofono-leave(a)ofono.org
> 

Regards,
-Denis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Documentation on when to free structures in/after callbacks
  2021-07-28 17:31 ` Denis Kenzior
@ 2021-07-30 13:21   ` ajlennon
  2021-07-30 14:20     ` Denis Kenzior
  0 siblings, 1 reply; 7+ messages in thread
From: ajlennon @ 2021-07-30 13:21 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 638 bytes --]

>I'm not sure what you mean? What exactly do you think needs freeing?

Strange. I am lookinga at the drivers and sometimes unref functions are provided into other functions and sometimes not

        if (g_at_chat_send(nmd->chat, "AT+QENG=\"servingcell\"", qeng_prefix,
                                qeng_cb, cbd, req_cb_data_unref) == 0) {


Then these CALLBACK_WITH_FAILURE and CALLBACK_WITH_SUCCESS calls sometimes seem to have free functions after them and sometimes not.

I guess I am not understanding then.... Will continue look at this. I just thought there might be sometime in docs about the flow.

Regards,
-Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Documentation on when to free structures in/after callbacks
  2021-07-30 13:21   ` ajlennon
@ 2021-07-30 14:20     ` Denis Kenzior
  2021-07-30 14:24       ` ajlennon
  0 siblings, 1 reply; 7+ messages in thread
From: Denis Kenzior @ 2021-07-30 14:20 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

Hi Alex,

On 7/30/21 8:21 AM, ajlennon(a)dynamicdevices.co.uk wrote:
>> I'm not sure what you mean? What exactly do you think needs freeing?
> 
> Strange. I am lookinga at the drivers and sometimes unref functions are provided into other functions and sometimes not

So you're talking about callbacks in the driver itself, not actually crossing 
the core<->driver boundary.

There aren't really any docs, but g_at_chat acts just like any other GLib based 
library (or many main-loop driven C libraries for that matter), so pretty much 
the same rules apply.  I can give you some hints...

> 
>          if (g_at_chat_send(nmd->chat, "AT+QENG=\"servingcell\"", qeng_prefix,
>                                  qeng_cb, cbd, req_cb_data_unref) == 0) {
> 
> 

Under the hood, g_at_chat_send puts a request on the queue.  And the request 
might not be processed until some time later.  It can also be interrupted and 
cleaned up without the callback being called (if the hardware gets pulled for 
example).  So if you're passing any allocated userdata, you need to provide a 
'destroy' method to de-allocate it, otherwise it will leak.

In your example, cbd is userdata and req_cb_data_unref is the destroy method, 
respectively.

Oh, and req_cb_data is a common pattern.  It is used where you need to run 
several commands in sequence (think of it as a transaction) and the allocated 
userdata needs to be cleaned up even if the transaction is interrupted.

> Then these CALLBACK_WITH_FAILURE and CALLBACK_WITH_SUCCESS calls sometimes seem to have free functions after them and sometimes not.
> 

There is definitely a reason why they do.  Hopefully the explanation above helps.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Documentation on when to free structures in/after callbacks
  2021-07-30 14:20     ` Denis Kenzior
@ 2021-07-30 14:24       ` ajlennon
  2021-07-30 14:30         ` Denis Kenzior
  0 siblings, 1 reply; 7+ messages in thread
From: ajlennon @ 2021-07-30 14:24 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

Thanks Denis that helps. I see I need to learn a bit and get my head around this.

For example I find this really confusing:

  cbd = req_cb_data_ref(cbd);
        if (g_at_chat_send(nmd->chat, "AT+CESQ", cesq_prefix,
                                cesq_cb, cbd, req_cb_data_unref) == 0) {
                CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
                req_cb_data_unref(cbd);
        }

This does some kind of allocation - ok
This makes the call - ok fine
Provides the unref function into the call - ok so far

But then I'm thinking if the call _succeeds_ our unref gets called?
But if our call _fails_ then we have to do the unref ourselves?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Documentation on when to free structures in/after callbacks
  2021-07-30 14:24       ` ajlennon
@ 2021-07-30 14:30         ` Denis Kenzior
  2021-07-30 15:12           ` ajlennon
  0 siblings, 1 reply; 7+ messages in thread
From: Denis Kenzior @ 2021-07-30 14:30 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

Hi Alex,

On 7/30/21 9:24 AM, ajlennon(a)dynamicdevices.co.uk wrote:
> Thanks Denis that helps. I see I need to learn a bit and get my head around this.
> 
> For example I find this really confusing:
> 
>    cbd = req_cb_data_ref(cbd);
>          if (g_at_chat_send(nmd->chat, "AT+CESQ", cesq_prefix,
>                                  cesq_cb, cbd, req_cb_data_unref) == 0) {
>                  CALLBACK_WITH_FAILURE(cbd->cb, cbd->data);
>                  req_cb_data_unref(cbd);
>          }
> 
> This does some kind of allocation - ok
> This makes the call - ok fine
> Provides the unref function into the call - ok so far
> 

So in this particular case, g_at_chat_send() returning 0 means the command could 
not be scheduled for whatever reason (usually the tty port is dead).  By 
convention, failures do not side-effect.  So you have to free any allocated 
resources manually.  Hence the unref here.

> But then I'm thinking if the call _succeeds_ our unref gets called?

Yes.  destroy callback is called in every circumstance once g_at_chat_send() 
returns a success.  So it will get called after cesq_cb(), or if the request is 
cleaned up early (cesq_cb() never called)

> But if our call _fails_ then we have to do the unref ourselves?

Right, the 'no-side-effect' part applies.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Documentation on when to free structures in/after callbacks
  2021-07-30 14:30         ` Denis Kenzior
@ 2021-07-30 15:12           ` ajlennon
  0 siblings, 0 replies; 7+ messages in thread
From: ajlennon @ 2021-07-30 15:12 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 89 bytes --]

Thanks for that Denis - that helps me get it much clearer in my head!

Cheers, 

Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-30 15:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28 16:32 Documentation on when to free structures in/after callbacks ajlennon
2021-07-28 17:31 ` Denis Kenzior
2021-07-30 13:21   ` ajlennon
2021-07-30 14:20     ` Denis Kenzior
2021-07-30 14:24       ` ajlennon
2021-07-30 14:30         ` Denis Kenzior
2021-07-30 15:12           ` ajlennon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).