Page MenuHomePhabricator

eldbus: properly cleanup local variable during destruction.
ClosedPublic

Authored by cedric on Oct 4 2019, 5:26 PM.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
cedric created this revision.Oct 4 2019, 5:26 PM
cedric requested review of this revision.Oct 4 2019, 5:26 PM

May I inquire why you replace eina_stringshare_del with a NULL parametered eina_stringshare_replace? The former is very clear on what the code should do, delete the string, the latter requires much more thought to go about what this actually should do.

ProhtMeyhet added inline comments.Oct 4 2019, 11:14 PM
src/lib/eldbus/eldbus_signal_handler.c
242

shouldn't it be exists instead of exit?

bu5hm4n added inline comments.Oct 5 2019, 1:42 AM
src/lib/eldbus/eldbus_model.c
62

I think i do not understand that change.

86

I think i do not understand that here as well.

cedric added a comment.Oct 5 2019, 6:27 AM

May I inquire why you replace eina_stringshare_del with a NULL parametered eina_stringshare_replace? The former is very clear on what the code should do, delete the string, the latter requires much more thought to go about what this actually should do.

Replace does free and null pointer in one operation. It should be standard practice in any object that could live long enough to trigger code that might access this pointer (think other callbacks).

cedric added inline comments.Oct 5 2019, 6:30 AM
src/lib/eldbus/eldbus_model.c
86

The unref can trigger asynchronous operation which could trigger access in other function where this pointer could be accessed. Seeing it to null before destroying it prefer this rogue access.

Same for the above one.

src/lib/eldbus/eldbus_signal_handler.c
242

Indeed, didn't check the comment

Looks good, but since this is fixing (and maybe) causing new issues, can you sent this through travis ?

May I inquire why you replace eina_stringshare_del with a NULL parametered eina_stringshare_replace? The former is very clear on what the code should do, delete the string, the latter requires much more thought to go about what this actually should do.

Replace does free and null pointer in one operation. It should be standard practice in any object that could live long enough to trigger code that might access this pointer (think other callbacks).

Thank you for the clarification. It seems I just don't agree with the api then :-)

zmike accepted this revision.Oct 7 2019, 7:03 AM

hurray recursion

This revision is now accepted and ready to land.Oct 7 2019, 7:03 AM
This revision was automatically updated to reflect the committed changes.