[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