[XForms] Re: JPEG/EXIF support?

[Clive A Stubbings]

Hi Angus,

I built the original fix a long time ago, possibly on one of the really
early open releases. It is always possible that it has been subsequently
fixed elsewhere.

The old flimage_dup did 

        FL_IMAGE *
        flimage_dup_(FL_IMAGE * sim, int pix)
        {
        ...
                FL_IMAGE *im = flimage_alloc();
                ...
                memcpy(im, sim, sizeof(*im));


Which seems a bit iffy.. I've just checked - thats is also how it is
in 1.0. Can you check the current version?


Cheers
Clive

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