GNOME Bugzilla – Bug 697791
gst-inspect: shows action signal return pointers as pointers
Last modified: 2013-04-13 11:04:45 UTC
Created attachment 241251 [details] [review] Patch to solve the bug When a signal or action returns a pointer, gst-inspect displays just the type name and does not indicate that the return type is a pointer. For example: gst-inspect-1.0 rtpptdemux shows: ... "request-pt-map" : GstCaps user_function (GstElement* object, ... but it must show ... "request-pt-map" : GstCaps * user_function (GstElement* object, ... I attach a patch that solves this problem. (The patch applies over master and 1.0 branches)
Thanks, applied with some changes (changed it to check explicitly for known pointer types). There is no guarantee that a fundemantal type is not a pointer, and a flags type is also not a pointer, for example. Better whitelist known pointer types. commit 9e98492e12d5df6029a876e3797040e90a8237fd Author: Jose Antonio Santos Cadenas <santoscadenas@gmail.com> Date: Thu Apr 11 14:54:32 2013 +0200 gst-inspect: add pointer mark to signal and action return types that are pointers When the return type of a signal or action is a pointer, it should have an asterisk to mark it as such. https://bugzilla.gnome.org/show_bug.cgi?id=697791
I've tested your changes and I get this: "pointer" : gpointer * user_function (GstElement* object); instead of this: "pointer" : gpointer user_function (GstElement* object); I think that the asterisk should not be printed if type is a pointer. I have a patch but I'm not sure sure if I have to open a new bug or reopen this. Additionally, is it OK to also add the patches to the 1.0 branch?
Just attach your patch here please (and re-open or not). What element is this for? gpointer is an awful return type that should never be used in an API. What's the actual GType in this case? G_TYPE_POINTER or a derived type named "gpointer"? I suppose the problem is that for registered pointer types you don't really know if the typedef already includes the * bit or not. I would probably just special-case gpointer then, since it is kind of special.
Ok, I'll mark bug as unconfirmed, otherwise I cannot attach a new file. I don't know any element returning this, but I've modified one to do it just for testing. The type I used is G_POINTER directly.
Created attachment 241324 [details] [review] gst-inspect: Do not print an asterisk if the return type is gpointer Sorry, I've just see that I didn't need to mark bug as unconfirmed to attach a new file. This patch do not treat G_POINTER as an special case, just adds the asterisk only if the type is boxed of g_object.
Comment on attachment 241324 [details] [review] gst-inspect: Do not print an asterisk if the return type is gpointer But why? This doesn't seem right to me. It fixes a corner case that should never happen in practice (if it does the element needs to be fixed), and isn't right for the still rather unlikely but possible cases of derived pointer types, where you would register a pointer type named 'FooHandle' for example, and in those cases you want to show the '*' as well I think.
Probably I'm wrong but I think that if your register a new pointer type, your type itself is a pointer and you don't need the '*'. If I'm wrong, I completely agree with you and this second patch is not necessary.
Sorry, I've seen that, for example, "GstStaticCaps" is a pointer type and need the "*". Forget the second patch. Sorry for repeating but, is it OK to also apply the patch to 1.0 branch? Thanks.
Well, you are right of course, so let's just fix it then: commit 21e584696d9d63e0a35003e88687764e2d6f602e Author: Tim-Philipp Müller <tim@centricular.net> Date: Sat Apr 13 12:00:12 2013 +0100 gst-inspect: only add a '*' for non-'gpointer' pointers Spotted by Jose Antonio Santos Cadena. https://bugzilla.gnome.org/show_bug.cgi?id=697791 I will cherry-pick this into 1.0 (we usually only cherry-pick things into 1.0 that are deemed 'important' so that the diff to the previous version is small).