<Gimp/G>Scanner leaking fd - by design ?
This discussion is connected to the gimp-developer-list.gnome.org mailing list which is provided by the GIMP developers and not related to gimpusers.com.
This is a read-only list on gimpusers.com so this discussion thread is read-only, too.
<Gimp/G>Scanner leaking fd - by design ? | Hans Breuer | 20 Jun 13:44 |
<Gimp/G>Scanner leaking fd - by design ? | Sven Neumann | 20 Jun 15:05 |
<Gimp/G>Scanner leaking fd - by design ? | Hans Breuer | 20 Jun 16:32 |
<Gimp/G>Scanner leaking fd - by design ?
[
Sent to gtk-devel w/o response :
The patch below fixes it on the Gimp level, ok to apply ?
]
In glib/gscanner.c there are various places where scanner->input_fd is set to -1
Current Gimp cvs (compiled on win32) triggers the one in g_scanner_get_char() and as a result looses it's reference to the input file.
Is it an error in the usage of GScanner or should the
input_fd simply not be used as persitent during the
scanning phase ?
Shouldn't there be at least a g_warning when GScanner
destroys the input_fd reference ?
Thanks in advance, Hans
--- from-cvs/gimp/app/config/gimpscanner.c Thu Apr 24 13:38:02 2003
+++ my-gtk/gimp/app/config/gimpscanner.c Thu Jun 19 19:16:28 2003
@@ -54,6 +54,13 @@
gboolean is_error);
+typedef struct _GimpScannerUserData GimpScannerUserData;
+struct _GimpScannerUserData
+{
+ GError **error;
+ int fd;
+};
+
/* public functions */
GScanner *
@@ -62,6 +69,7 @@
{
gint fd;
GScanner *scanner;
+ GimpScannerUserData *user_data;
g_return_val_if_fail (filename != NULL, NULL);
@@ -82,7 +90,11 @@
g_scanner_input_file (scanner, fd);
- scanner->user_data = error;
+ user_data = g_new(GimpScannerUserData, 1);
+ user_data->error = error;
+ user_data->fd = fd;
+
+ scanner->user_data = user_data;
scanner->msg_handler = gimp_scanner_message;
scanner->input_name = g_strdup (filename);
@@ -98,9 +110,13 @@
void
gimp_scanner_destroy (GScanner *scanner)
{
+ GimpScannerUserData *user_data = scanner->user_data;
+
g_return_if_fail (scanner != NULL);
- close (scanner->input_fd);
+ if (close (user_data->fd) != 0)
+ g_warning ("Failed to close '%s' : %s", scanner->input_name,
g_strerror (errno));
+ g_free (scanner->user_data);
g_free ((gchar *) scanner->input_name);
g_scanner_destroy (scanner);
}
@@ -344,12 +360,12 @@
gchar *message,
gboolean is_error)
{
- GError **error = scanner->user_data;
+ GimpScannerUserData *user_data = scanner->user_data;
/* we don't expect warnings */
g_return_if_fail (is_error);
- g_set_error (error,
+ g_set_error (user_data->error,
GIMP_CONFIG_ERROR, GIMP_CONFIG_ERROR_PARSE,
_("Error while parsing '%s' in line %d:\n%s"),
scanner->input_name, scanner->line, message);
-------- Hans "at" Breuer "dot" Org ----------- Tell me what you need, and I'll tell you how to get along without it. -- Dilbert
<Gimp/G>Scanner leaking fd - by design ?
Hi,
Hans Breuer writes:
Sent to gtk-devel w/o response
Did you file a bug-report? This is unlikely ever to be fixed without a bug-report.
Current Gimp cvs (compiled on win32) triggers the one in g_scanner_get_char() and as a result looses it's reference to the input file.
Actually, I'd be interested to hear how exactly we are triggering this. When I saw your mail on gtk-devel I looked at the code in question but I could not find out what is going wrong.
+typedef struct _GimpScannerUserData GimpScannerUserData;
I always wondered who invented the term user_data, can't it just be GimpScannerData? Apart from that (and a missing blank) the patch looks OK. However I'd rather see this fixed in GLib. So it should probably be marked with a #warning and removed later when we can depend on a fixed version of GLib.
Sven
<Gimp/G>Scanner leaking fd - by design ?
At 15:05 20.06.03 +0200, Sven Neumann wrote:
Hi,
Hans Breuer writes:
Sent to gtk-devel w/o response
Did you file a bug-report? This is unlikely ever to be fixed without a bug-report.
Current Gimp cvs (compiled on win32) triggers the one in g_scanner_get_char() and as a result looses it's reference to the input file.
Actually, I'd be interested to hear how exactly we are triggering this. When I saw your mail on gtk-devel I looked at the code in question but I could not find out what is going wrong.
I'm not sure if it is a win32 only thing. Without very deep understanding of GScanner I thought it always runs to no-more-chars-avail in g_scanner_get_char(). Maybe you can reproduce by simply checking the return value of close() in gimp_scanner_destroy()
To reproduce I simply start and quit The Gimp. It happens on all file in ~/.gimp13/tool-options and documents, sessionrc. The latter can not be written on exit cause they are still open (on win32). This already gave an error message.
+typedef struct _GimpScannerUserData GimpScannerUserData;
I always wondered who invented the term user_data, can't it just be GimpScannerData? Apart from that (and a missing blank) the patch looks OK. However I'd rather see this fixed in GLib. So it should probably be marked with a #warning and removed later when we can depend on a fixed version of GLib.
Given the other use cases of GScanner I looked at (in gtk) I'm not sure if input_fd is supposed to stay valid - they all maintain their fd independent of it. But one could still call it a documentation bug of GLib :-)
Hans
-------- Hans "at" Breuer "dot" Org ----------- Tell me what you need, and I'll tell you how to get along without it. -- Dilbert