Hi Werner

> thanks again for your comments and for the patches. As usual I already  
> worked on a fix and in fact made nearly the identical changes to the  
> code as you suggested (commited to svn just now). About the merging  
> the options - this we should discuss here on the list. I conclude now  
> everything and ask the list for comments:
> 
> Before the commit:
>    - every time plsetopt was called the saved (command line options  
> and options set with plsetopt) *driver* options were forgotten  
> (leading to a memory leak) even if you didn't set driver options but  
> other options like "geometry"

That is what I concluded, yes.

> After the commit:
>    - every time driver option are set (at command line or with  
> plsetopt()) driver options set before are "forgotten" (without memory  
> leak). If other options (e.g. plsetopt("geometry","800x600")) are set  
> which are not driver specific the driver options are still "remembered".

That's certainly better, but as you said:

> The only problem now is, that you can't delete already set drv_opt  
> without setting new ones - but some drivers don't take any specific  
> driver options.

The ability to delete existing options I think would be helpful if we can
work out a way to make it happen without contradicting other assumptions
within the PLplot option parsing mechanism.

> Jonathan, your merge code contradicts the driver  
> options policy, since as far as I understand it, the following code >>  
> plsetopt( "drvopt", "text" ); << would cause the option text to be  
> deleted if it was already set. But such code should actually set the  
> driver option text to the value 1.

Um no, that was certainly not the intention.  Since no "=" sign is given in
your example the value should be set to 1 before opt_drvopt_merge() is
called.  Thus the original behaviour regarding default values is retained.

What *will* happen is if you instead call

  plsetopt( "drvopt", "text=" );

then that will trigger a delete operation on any existing driver option
called "text".

This behaviour comes about due to the structure of opt_drvopt().  Before
calling opt_drvopt_merge() "fl" is checked and if it's zero then a default
value is set.  fl will be 0 only if no "=" has occurred within the option
specification, and it is this behaviour that the delete functionality
exploits.

> Also, I'm not sure if merging is needed (it was also my first thought in
> the beginning), since the options for the different drivers are not
> identical anyway, so for a new driver you need to clean the options table
> anyway.

The reason why merging appealed to me is that it could simplify program flow
in some circumstances.  If one is setting device options based on a number
of conditionals, a merging plsetopt() means this can happen cumulatively. If
it's "all or nothing" then one has to keep adding options to a string buffer
and then do a single plsetopt() at the end.  Furthermore, if merging is not
done then I *think* any device options given on the command line will be
immediately lost when plsetopt() is called.

Merging is also closely related to the idea of changing the value of an
existing options (something else my patch allows).  If we don't do merging
one has to keep track of all options currently in use and reconstruct an
entire option string whenever one needs to change only one option.  Given
the alternative of just re-setting the option concerned, the resulting code
would be rather messy.

To help put some of this into context, I came across this whole issue while
working on changes which give the ability to use the xcairo driver to draw
into an X Drawable rather than a managed window.  I hope to post a
proof-of-concept patch in a few days once I've ironed out one or two rough
edges.  

> So, I'm not sure if it makes sense to merge options. Still now it's  
> not possible to clean the drv_opt table which I think is necessary.

To clean the option table we could just special-case a call of

  plsetopt( "drvopt", "" );

to mean "clean the driver options table".  Or we could reserve the "delete"
pseudo-option to trigger this behaviour.
  
Regards
  jonathan

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Reply via email to