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 434535 - printoperation's create_surface doesn't check temp file creation for success
printoperation's create_surface doesn't check temp file creation for success
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Printing
unspecified
Other Linux
: Normal minor
: ---
Assigned To: gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2007-04-30 09:01 UTC by Christian Persch
Modified: 2008-07-01 05:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch checking temp file creation success. (2.15 KB, patch)
2008-06-17 13:52 UTC, Marek Kašík
needs-work Details | Review
Reworked patch. (2.94 KB, patch)
2008-06-18 10:50 UTC, Marek Kašík
none Details | Review
Re-reworked patch :) (3.01 KB, patch)
2008-06-18 15:10 UTC, Marek Kašík
none Details | Review
Patch without the typo. (3.01 KB, patch)
2008-06-27 16:36 UTC, Marek Kašík
committed Details | Review

Description Christian Persch 2007-04-30 09:01:55 UTC
_gtk_print_operation_platform_backend_create_preview_surface creates a temp file, but doesn't check whether the operation succeeded (fd >= 0) before proceeding to use the FD.
Comment 1 Matthias Clasen 2007-04-30 17:24:20 UTC
Unfortunately, there is no really good way to handle that failure right now.
Comment 2 Christian Persch 2007-04-30 19:39:46 UTC
Maybe _gtk_print_operation_platform_backend_create_preview_surface could return NULL, gtk_print_operation_preview_handler check it and return FALSE, and gtkprintoperation.c:print_pages signal the error, instead of aborting with g_error?
Comment 3 Matthias Clasen 2007-04-30 19:43:43 UTC
Yes, thats probably what needs to happen here. 
Comment 4 Marek Kašík 2008-06-17 13:52:50 UTC
Created attachment 112908 [details] [review]
Patch checking temp file creation success.

Hi all,
this patch implements checking of temp file creation success.

  Marek
Comment 5 Matthias Clasen 2008-06-17 13:57:23 UTC
Looks mostly ok, but you should separate the programming error
gtk_print_context_get_cairo_context (priv->print_context) == NULL
from the runtime error !handled
and keep the g_error () for the programming error
Comment 6 Marek Kašík 2008-06-18 10:50:29 UTC
Created attachment 112970 [details] [review]
Reworked patch.

Hi,
this is reworked patch. It is reworked according to previous comment.

  Marek
Comment 7 Christian Persch 2008-06-18 12:22:30 UTC
+          gtk_dialog_run (GTK_DIALOG (error_dialog));

Print operations may run async, in which case they must never block. Using gtk_dialog_run is therefore unacceptable.
I think just showing the dialogue and connecting "response" to gtk_widget_destroy would do here.
Comment 8 Marek Kašík 2008-06-18 15:10:50 UTC
Created attachment 112985 [details] [review]
Re-reworked patch :)

Hi Christian,
thank you for your comment. I reworked the patch according to it.

  Marek
Comment 9 Matthias Clasen 2008-06-18 15:22:32 UTC
Thanks, looks good now. One stylistic nit is that you may want to say
"a temporary file" not just "temporary file".
Comment 10 Marek Kašík 2008-06-18 15:55:10 UTC
Yes, that is exactly what I want to say. :-)
Comment 11 Marek Kašík 2008-06-27 16:36:27 UTC
Created attachment 113538 [details] [review]
Patch without the typo.
Comment 12 Matthias Clasen 2008-07-01 05:39:05 UTC
        * gtk/gtkprintoperation-unix.c
        (_gtk_print_operation_platform_backend_create_preview_surface):
        Handle failure to create temp file by returning NULL.

        * gtk/gtkprintoperation.c (gtk_print_operation_preview_handler):
        Return FALSE if surface creation fails.

        (print_pages): If the preiew signal is not handled, show an
        error dialog.