After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 789319 - Fix up GIR annotations
Fix up GIR annotations
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-22 12:40 UTC by Arun Raghavan
Modified: 2018-01-27 10:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
devicemonitor: Return NULL instead of FALSE (1023 bytes, patch)
2017-10-22 12:40 UTC, Arun Raghavan
committed Details | Review
devicemonitor: Don't return NULL when listing providers (1.11 KB, patch)
2017-10-22 12:40 UTC, Arun Raghavan
rejected Details | Review
deviceprovider: Don't return a NULL list of hidden providers (1.14 KB, patch)
2017-10-22 12:40 UTC, Arun Raghavan
rejected Details | Review
meta: Don't return NULL for zero-length gchar** array (718 bytes, patch)
2017-10-22 12:40 UTC, Arun Raghavan
rejected Details | Review
gst: Fix up a bunch of GIR annotations (48.63 KB, patch)
2017-10-22 12:41 UTC, Arun Raghavan
committed Details | Review

Description Arun Raghavan 2017-10-22 12:40:28 UTC
The first few patches make returned list values easier from a client
perspective (and also makes the annotations correct in the process).

The last is a fixup of several return value nullable annotation fixups in
gst/.
Comment 1 Arun Raghavan 2017-10-22 12:40:34 UTC
Created attachment 362046 [details] [review]
devicemonitor: Return NULL instead of FALSE

Same effect, meaning is clearer.
Comment 2 Arun Raghavan 2017-10-22 12:40:39 UTC
Created attachment 362047 [details] [review]
devicemonitor: Don't return NULL when listing providers

This should make client code simpler, as it doesn't have to do an extra
NULL check when the number of providers is 0.
Comment 3 Arun Raghavan 2017-10-22 12:40:46 UTC
Created attachment 362048 [details] [review]
deviceprovider: Don't return a NULL list of hidden providers

Should make things simpler from a client API perspective, avoiding the
need to check the return value for the zero-length case.
Comment 4 Arun Raghavan 2017-10-22 12:40:53 UTC
Created attachment 362049 [details] [review]
meta: Don't return NULL for zero-length gchar** array

This should make things easier for API users.
Comment 5 Arun Raghavan 2017-10-22 12:41:00 UTC
Created attachment 362050 [details] [review]
gst: Fix up a bunch of GIR annotations

This is mostly on nullable return values, and some other minor ones that
I ran across.
Comment 6 Sebastian Dröge (slomo) 2017-10-25 11:03:21 UTC
Comment on attachment 362049 [details] [review]
meta: Don't return NULL for zero-length gchar** array

This changes API. There might be code that just checks for NULL instead of tags[0]==NULL.
Comment 7 Sebastian Dröge (slomo) 2017-10-25 11:07:09 UTC
Comment on attachment 362050 [details] [review]
gst: Fix up a bunch of GIR annotations

You make some constructors (pad, ghostpad) return not nullable anymore, but the docs say that they can return NULL in error cases. Intentional? Why?

Otherwise looks good
Comment 8 Tim-Philipp Müller 2017-11-01 19:10:54 UTC
(In reply to Sebastian Dröge (slomo) from comment #6)
> Comment on attachment 362049 [details] [review] [review]
> meta: Don't return NULL for zero-length gchar** array
> 
> This changes API. There might be code that just checks for NULL instead of
> tags[0]==NULL.

Also, what's the rationale for this change? To me NULL for empty arrays seems easier, and it's also what pretty much all other functions returning arrays do.
Comment 9 Tim-Philipp Müller 2018-01-27 10:32:55 UTC
Comment on attachment 362047 [details] [review]
devicemonitor: Don't return NULL when listing providers

I strongly dislike this. I don't think we should change it :)
Comment 10 Tim-Philipp Müller 2018-01-27 10:33:19 UTC
Comment on attachment 362048 [details] [review]
deviceprovider: Don't return a NULL list of hidden providers

Same
Comment 11 Tim-Philipp Müller 2018-01-27 10:33:40 UTC
Comment on attachment 362046 [details] [review]
devicemonitor: Return NULL instead of FALSE

commit 41a59cddb6f294cecd367bfe2827a535a8690865 (HEAD -> master)
Author: Arun Raghavan <arun@arunraghavan.net>
Date:   Sat May 27 05:19:20 2017 +0530

    devicemonitor: Return NULL instead of FALSE
    
    Same effect, meaning is clearer.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=789319
Comment 12 Tim-Philipp Müller 2018-01-27 10:44:39 UTC
commit b5028383ab9cc96e72595d193700a177a6587891 (HEAD -> master)
Author: Arun Raghavan <arun@arunraghavan.net>
Date:   Sun Oct 22 18:05:30 2017 +0530

    gst: Fix up a bunch of GIR annotations
    
    This is mostly on nullable return values, and some other minor ones that
    I ran across.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=789319
Comment 13 Tim-Philipp Müller 2018-01-27 10:45:35 UTC
Comment on attachment 362050 [details] [review]
gst: Fix up a bunch of GIR annotations

Pushed this minus the bits mentioned by Sebastian in comment 7 where you remove the nullable return value annotation.

diff --git a/gst/gstghostpad.c b/gst/gstghostpad.c
index 478d2742b..36068ccdb 100644
--- a/gst/gstghostpad.c
+++ b/gst/gstghostpad.c
@@ -769,7 +769,7 @@ set_target_failed:
  * Create a new ghostpad based on @templ, without setting a target. The
  * direction will be taken from the @templ.
  *
- * Returns: (transfer floating) (nullable): a new #GstPad, or %NULL in
+ * Returns: (transfer floating): a new #GstPad, or %NULL in
  * case of an error.
  */
 GstPad *
diff --git a/gst/gstpad.c b/gst/gstpad.c
index a5475ab66..c1d4cd64e 100644
--- a/gst/gstpad.c
+++ b/gst/gstpad.c
@@ -827,7 +827,7 @@ gst_pad_get_property (GObject * object, guint prop_id,
  * will be assigned.
  * This function makes a copy of the name so you can safely free the name.
  *
- * Returns: (transfer floating) (nullable): a new #GstPad, or %NULL in
+ * Returns: (transfer floating): a new #GstPad, or %NULL in
  * case of an error.
  *
  * MT safe.
@@ -849,7 +849,7 @@ gst_pad_new (const gchar * name, GstPadDirection direction)
  * will be assigned.
  * This function makes a copy of the name so you can safely free the name.
  *
- * Returns: (transfer floating) (nullable): a new #GstPad, or %NULL in
+ * Returns: (transfer floating): a new #GstPad, or %NULL in
  * case of an error.
  */
 GstPad *