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 780234 - gtk_print_job_set_page_ranges() has unclear ownership transfer
gtk_print_job_set_page_ranges() has unclear ownership transfer
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Printing
3.89.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2017-03-18 08:48 UTC by Kjell Ahlstedt
Modified: 2017-03-24 15:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: printjob: Clarify array ownership in gtk_print_job_set_page_ranges() (1.14 KB, patch)
2017-03-18 08:54 UTC, Kjell Ahlstedt
none Details | Review
patch: printjob: Clarify array ownership in gtk_print_job_set_page_ranges() (1.33 KB, patch)
2017-03-19 13:55 UTC, Kjell Ahlstedt
committed Details | Review

Description Kjell Ahlstedt 2017-03-18 08:48:18 UTC
gtk_print_job_set_page_ranges(GtkPrintJob *job, GtkPageRange *ranges,
gint_ranges) takes ownership of the GtkPageRange array, but this is not
mentioned in the documentation. The pointer is copied to job->priv->page_ranges
and the array is later freed by gtk_print_job_finalize().
Compare gtk_print_settings_set_page_ranges(), which is documented exactly the
same, but does not take ownership.
If gtk_print_job_set_page_ranges() is called more than once, the ownership
issue becomes really confusing, because gtk_print_job_set_page_ranges() does not
free a previously stored array.
Comment 1 Kjell Ahlstedt 2017-03-18 08:54:12 UTC
Created attachment 348217 [details] [review]
patch: printjob: Clarify array ownership in gtk_print_job_set_page_ranges()

This patch clarifies the ownership transfer. I'm not sure if it's the right way
to do it, though. I've not found any other functions with a transfer annotation
on an input parameter.
Comment 2 Matthias Clasen 2017-03-19 12:24:10 UTC
Review of attachment 348217 [details] [review]:

The commit message should certainly mention that a g_free call is added to fix a memory leak when set_page_ranges is used repeatedly.

::: gtk/gtkprintjob.c
@@ +795,3 @@
                                gint          n_ranges)
 {
+  g_free(job->priv->page_ranges);

space before (, please
Comment 3 Kjell Ahlstedt 2017-03-19 13:55:26 UTC
Created attachment 348258 [details] [review]
patch: printjob: Clarify array ownership in gtk_print_job_set_page_ranges()

Updated patch.
Comment 4 Matthias Clasen 2017-03-23 23:00:28 UTC
Review of attachment 348258 [details] [review]:

looks good now, thanks
Comment 5 Kjell Ahlstedt 2017-03-24 15:45:47 UTC
Comment on attachment 348258 [details] [review]
patch: printjob: Clarify array ownership in gtk_print_job_set_page_ranges()

Pushed to the master branch