RSS/Atom feed Twitter
Site is read-only, email is disabled

<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.

3 of 3 messages available
Toggle history

Please log in to manage your subscriptions.

<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
Hans Breuer
2003-06-20 13:44:57 UTC (almost 21 years ago)

<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

Sven Neumann
2003-06-20 15:05:04 UTC (almost 21 years ago)

<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

Hans Breuer
2003-06-20 16:32:22 UTC (almost 21 years ago)

<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