[XForms] Re: JPEG/EXIF support?

Angus Leeming angus.leeming at btopenworld.com
Thu Nov 27 06:49:24 EST 2003


On Thursday 27 November 2003 2:27 am, Clive A Stubbings wrote:
> > > It fixes a bug in flimage_dup() ,
> >
> > I'm not going to apply this without some discussion. Feel free to explain
> > the rationale.
>
> Well, its a while ago, but if memory serves, I found that if I dup'd an
> image then later free'd both copies I got a crash. The crash was because
> the first free also closed the files and the second one tried to do the
> same again, but the fp's were no longer valid.
>
> There needs to be only one reference to the fp.. Either it goes in a
> shared struct that both 'images' point to, or only one image can be
> associated with the file. I'm not sure that 2 'images' and one file
> really makes sense anyway.
>
> FWIW: I duped the image because I wanted to have the original and a
> processed copy, so I made a dup and then processed it.. When I'd
> finished with both images I free'd both.. Seemed reasonable to me.

And to me too. I do something similar in LyX. Here are the relvant parts of my 
wrapper class. My question to you is, "why doesn't this trigger the crash"? 
If I make two copies of the image and then invoke flimage_free in the 
destructor, why don't I run into your problem?

class xformsImage {
public:
	xformsImage(): image_(0) {}
	xformsImage(xformsImage const & other) : image_(0) {
		if (other.image_) {
			image_ = flimage_dup(other.image_);
			image_->u_vdata = this;
		}
	}

	~xformsImage() {
		if (image_)
			flimage_free(image_);
	}
private:
	/// The xforms container.
	FL_IMAGE * image_;
};

Ahhhhhhh. Because I use a callback function to monitor the loading process and 
explicitly close the FILE pointers when loading is complete.

void xformsImage::statusCB(string const & status_message)
{
	if (status_message.empty())
		return;

	if (prefixIs(status_message, "Done Reading")) {
		if (image_)
			flimage_close(image_);
		finishedLoading(true);
	}
}

Anyway, I see the need for your patch or for something like it. 

Perhaps it would be better to not copy the file pointers in flimage_dup in the 
first place? Hmmmmmmmm. flimage_dup does NOT copy the FILE pointers. The only 
place that they are set is in identify_image which in turn is invoked only by 
flimage_open (in turn invoked by flimage_load). The point is that flimage_dup 
does not invoke any of these routines.

Conclusion: I don't see the need for the patch.
No doubt I've got my logic wrong somewhere, so I'd be grateful if you'd have a 
look at the current xforms sources too...



> > > and makes jpeg + gif reading better.
> >
> > I've applied both these parts to my local tree, albeit with some
> > re-factoring to get rid of the goto and some signed-unsigned comparson
> > warnings. Could you check to see that all is still as you intended
> > (attached). If you could cast your eye over the ChangeLog entry and maybe
> > rename 'flush_buffer' to something that makes sense to you, that'd be
> > great too...
>
> Had a fair look at it - seems OK on paper. flush_buffer is fine. The
> function flushes completed lines from the working buffer.

And Michal says it works, so I have committed it. Many thanks for the patch.

> (If you don't like the goto, with hindsight, an else might have been better
> ;-)

Let's just say I have a pathological hatred of 'goto'. Something to do with 
maintaining some old Fortran66 code.

Regards,
Angus




More information about the Xforms mailing list