Issue with lv_obj_del

[embeddedt]

Trees grow and shrink thru usage, and all that growing/pruning/grafting/etc could leave dangling stuff.
So if I had a use case where I had a tree that I wanted to create, use a lot, then delete. I could see using the lv_obj_del_safe on the root node’s lv_obj, and clean out the children safely.

I guess there is no harm in adding lv_obj_del_safe for a case like that.

As a personal preference, though, I would prefer to fix the code in the event that someone left dangling pointers, rather than relying on the library to spot it for me. But that’s just my opinion.

[kisvegabor]

As a personal preference, though, I would prefer to fix the code in the event that someone left dangling pointers, rather than relying on the library to spot it for me. But that’s just my opinion.

I agree. If you relied on the library will spot the dirty pointer then we would need lv_label_create_safe() (what if the parent is invalid) lv_label_set_text_safe (what if the label is invalid) and so on.

IMO, helping the user to find the issue is more reasonable.

That means that an application would be significantly slowed down when validity checks are in place. This is great for development (and when you are trying to find a bug), but would probably have a significantly negative affect on performance.

I agree with this too.

[rohmer]

@kisvegabor I disagree with this assertion:
" I agree. If you relied on the library will spot the dirty pointer then we would need lv_label_create_safe() (what if the parent is invalid) lv_label_set_text_safe (what if the label is invalid) and so on."

2 points:
First of all - I am not suggesting reliance on the library, not at all (To ref @embeddedt’s comment, I agree totally. That is why I would never use this code path in development. Thats where you WANT to see bugs). What I am suggesting, is in complicated, long running systems, bugs surface that you had no clue you were there. Back to the code path comment.
Second - The application would be slowed in the event of this code path, and I would not suggest it for every case, but in the case of a complicated thing, that changed a lot, I would indeed want to use it for safety.

[rohmer]

agreed, I too would want to fix the code as well. And this would be a good place for telemetry, to tell me that the safe delete spotted something that was amiss.

Rather than just crash.

[kisvegabor]

However, issues can occur in other functions too. Why do we pick only lv_obj_del?
Having issues with lv_obj_get_parent() or lv_obj_get_width is similarly likely.

If the goal is to leave certain protection in production code too it’d be possible to make the checks configurable. E.g.:

#define LV_DEBUG_CHECK_NULL           1
#define LV_DEBUG_CHECK_OBJ_VALIDITY   1
#define LV_DEBUG_CHECK_OBJ_TYPE       1

[rohmer]

Well my thinking was I would not want to do it on all calls to delete or whatever, just in specific instances

[embeddedt]

just in specific instances

Which makes me think it would be better to add lv_obj_valid (and possibly lv_obj_check_type), both of which would return bool and could be used in a project-specific assertion.

[rohmer]

That I think is best.

Quick question for you, how do you find an objects subtype at runtime (I havent dug into this yet).

For exp:
lv_obj_t *someType

How do I find at runtime if it is an lv_obj_t or lv_button or whatever?

I suppose I could do dynamic casting or rtti, but that seems like a sledgehammer

[embeddedt]

https://docs.littlevgl.com/en/html/object-types/obj.html#_CPPv415lv_obj_get_typeP8lv_obj_tP13lv_obj_type_t

[rohmer]

Now I just feel stupid, I should have RTFM’d

Thanks thought!
Ron

[kisvegabor]

Which makes me think it would be better to add lv_obj_valid (and possibly lv_obj_check_type ), both of which would return bool and could be used in a project-specific assertion.

Great idea. Do you think the “debug mode” (to perform checks on every function call) is still viable?

[rohmer]

@kisvegabor Debug mode on every function call may still be viable. Either way it will lead to a documentation “bug”

We will need to call out the functionality, maybe in a debugging section. I believe adding these options will make lvgl a lot easier to use for less experienced people

[kisvegabor]

We will need to call out the functionality, maybe in a debugging section. I believe adding these options will make lvgl a lot easier to use for less experienced people

Seeing if we are in Debug or not is platform dependent. IMO is should be enabled by default in the simulator (also add log entry to tell “we are in debug mode which is safe but slow”) but disabled in lv_conf_templ.h.

[rohmer]

That sounds good to me, or it could be tied to -DDEBUG, but that gets you into compiler semantics of which define that correctly, etc.

[kisvegabor]

Great! I can put together the basic engine next week.

[rohmer]

Sweet…

One day we should have a chat about some ideas I have and possible plans for 7.0…

I think lvgl is amazing (Hence why I am giving my time to it, sigh not enough of that), and I think with a few additions it could really be the best out there.

[kisvegabor]

Good to hear that! Please open a topic in the “Feature request” topic and list your ideas.

[kisvegabor]

@rohmer
Issues with styles like this are very common. To mitigate this, I’ll add a “style check” too.

[kisvegabor]

I’ve started to implement this in feat-debug branch.

Here is the API.
It used experimentally in lv_label.

Let me know what do you think.

[kisvegabor]

I’ve added all the required asserts and merged this feature to dev-6.1 branch.
In lv_conf.h you can enable/disable which asserts to use.