Date: 15 Feb 2001 09:29:42 -0500
From: Thomas Vogels
Subject: Re: [Rd] who frees dd and xd in X11_Open?
Prof Brian Ripley writes:
On 14 Feb 2001, Thomas Vogels wrote:
Hi, I'm not sure this is a bug in the code, the comment or my
thinking. So first try goes to r-devel... I find the following
comment in X11_Open () (src/unix/X11/devX11.c):
/* if we have to bail out with "error", then must free(dd) and free(xd) */
A couple lines down, there is:
if (!strncmp(dsp, "png::", 5)) {
FILE *fp;
#ifndef HAVE_PNG
error("No png support in this version of R");
#endif
if (!(fp = R_fopen(R_ExpandFileName(dsp+5), "w")))
error("could not open PNG file `%s'", dsp+6);
...
So what does the comment imply? If error jumps out to the toplevel
without giving X11DeviceDriver a chance to free dd and xd, then this
should be:
[...]
Well, the thinking was that people would not be trying to use facilities
on systems where they had not bothered to implement them, so what problem
was a small memory leak? They would only do it once, ever ....
uh, I guess I screwed up the example. Even more lines down, there is
no free accompanying the error ("could not open PNG file ..."). That
can happen more often and more memory would leak. But no, I wanted to
know this for another reason: I've played around with the X11 device
driver to implement a feature that allows me to embed the device into
another window. Opening and closing of x11()'s can happen often
during the execution of the program. But I'm still not interested in
the memory leak issue per se.
I've seen a segfault or two (not in the regular code) and right now I
blame it on Xevents arriving after the device is shut down. Now all
of a sudden the calls to free become very interesting. As I said at
the beginning of the email, I wanted to simply understand the comment,
understand what should be going on, not criticize for it not being
followed.
Although I got the question wrong, I guess, I got the right answer :-)
Thanks.
Regards,
-tom
In these cases it would be better to give a warning and return FALSE, when
the normal return mechanism will free xd and then dd. I will alter it to
that when I have tested it out.