That’s a “should never happen” assertion failure. If you are serious about handling it without undefined behavior, error must be set as well (and removing g_return*_if_fail from everywhere would be a much larger project IIRC).

Alternatively, if it is only linter-driven, some other code paths in the same function are using g_assert(sdata != NULL), and that seems to be acceptable by that linter.

@mitr, thank you for the comment.

I'm not sure if I understand correctly. I added new commit with g_assert - do you suggest this?
This also possible but it will change lu_dispatch behavior a bit (probably we do not care).

On the other hand the assert message is not lost this way.

I don’t have an opinion on g_assert vs. g_return_val_if_fail; the current use was motivated by something like the documentation in https://developer.gnome.org/glib/stable/glib-Warnings-and-Assertions.html but this (on an impossible error, log it but continue with undefined state) was always a bit of a dubious approach.

This is a coding style decision completely up to the current maintainers.

OTOH the original PR, just using an if and silently propagating undefined state, seemes to me to be clearly making things worse. The memory leak was a better outcome than that. (Either way, almost all callers would crash on expecting a set error value and seeing NULL, but with the previous code there would be a relevant error message before the crash.)

If you want to preserve the logging but make sure the data is freed, an explicit if+free+g_return_if_reached would be the minimal behavior change. Again, up to the current maintainers what they prefer.

(It might also make sense to get rid of the lu_dispatch thing and restructure the code to do something else, so that a lot of the weird abstraction and re-specialization, and the various impossible failure cases, go away. But that would be a fairly big project, and would also be absolutely up to the current maintainers.)