Re: [Plplot-devel] Exit calls and memory management

[phil r. <phi...@ya...>]

@Hazen and Alan
I think an email went AWOL from Hazen somewhere, but from Alan's reply I guess it probably said that we should not be rebasing public branches. I must confess that I think almost everything I have read about rebasing says do not rebase a public branch. I'm certainly no expert, but it does make me nervous that we are suggesting to fly in the face of the perceived wisdom. I think I would be much more comfortable with accepting that where we need to collaborate on things we and use public branches, we merge these instead of rebasing them. I know this makes for a slightly less tidy history, but such large changes don't happen often. Don't forget it isn't only us that can download a public branch - anyone can download from my repo, find and fix a bug, then make a push request - if we have to say sorry that history doesn't exist we can't accept your push, then it doesn't make us look great.
That is of course a very idealistic way of looking at things and I'm all for practicality over idealism. So if you still want to go for the rebase option then I have no problem with that. Just so long as we have considered the potential problems.

@Andrew. You are of course correct that these changes in themselves will not remove the exit calls, but my intention is that they will make failed memory allocations easier to deal with - even if it is just that by using an array's internal element count it will avoid buffer overruns even if memory allocation fails - i.e. it removes the need for each reallocation to be checked and the element count be either zero'd or reset to its old value.
Regarding the replacement to plexit. Two options were discussed at some point. Either 1) Use the plabort callback set by the user to report error messages or 2)Have an error code and message which we save and add a plgeterror() function to the API, to retrieve these values.
I am tending towards 1) for the following reasons:
a) Almost nobody ever bothers to actually check the error code, but if they have created a callback then they are more likely to make use of the error. (I am as guilty as that as anyone so I now pepper my code with exception throws to force at least some occurance if something fails)
b) For 2 we need to reset the error code for every api call. This isn't too bad, but we also have to make sure we never accidentally reset it by calling an API function internally - that will be a real pain to disentangle and enforce.
Some arguments for 2 though are
i) If we ever make plplot threadsafe then we need some way to make sure the plabort callback is also threadsafe - but I guess we would need that anyway
ii) I'm not sure what would happen if our callback threw an exception - it would probably leave the library in some unresolved state. But that is the case for the callback now too.

As you said the error code can be set and checked internally and a function can be used to allow it to be easily set by other bindings. Actually in a lot of cases Plplot can probably carry on regardless - providing the array size has been remembered correctly, hence the advantage of arrays which know their own size.

However I am certainly still open to suggestions - I think based on the other comments on this thread I will set up a github repository and people can see what I've done so far and comment and suggest as needed.

Phil

 

________________________________
 From: Alan W. Irwin <ir...@be...>
To: Hazen Babcock <hba...@ma...>; phil rosenberg <phi...@ya...> 
Cc: PLplot development list <plp...@li...>; Hezekiah M. Carty <he...@0o...> 
Sent: Monday, 22 September 2014, 23:00
Subject: Re: [Plplot-devel] Exit calls and memory management
  

On 2014-09-22 16:41-0400 Hazen Babcock wrote:

> On 9/22/2014 3:37 PM, Alan W. Irwin wrote:
>> On 2014-09-22 12:08-0400 Hazen Babcock wrote:
>> 
>>> I think creating a branch on github (or some other public repository) is
>>> the only way that you can proceed if you want others to see what you are
>>> doing. Though not perhaps ideal, you should be able to rebase master off
>>> a public branch.
>> 
>> Since our advice to Phil is completely contradictory, we should get
>> this important question of workflow decided for the case of
>> experimental topic development work (as opposed to the workflow for
>> our more usual less controversial topic development which is normally
>> pushed to origin master as soon as the changes by a developer pass his
>> own tests).  And once the workflow for this special case of
>> experimental development topics is decided, we should document it in
>> README.developers.
>> 
>> You are much more experienced with git than me.  However, I thought
>> that rebasing a public branch was always a bad idea for the reasons I
>> mentioned concerning disappearing commits.  I am positive a number of
>> resources I read when we were considering using a rebase workflow
>> mentioned this drawback to that approach.
>> 
>> Were those resources overstating the case?
>
> Probably not. However I don't really see a problem with someone pushing a 
> private branch of PLplot to a public repo and asking others to have a look 
> and make suggestions, as long as everyone else understands that it is "read 
> only". Actual collaboration on a branch is going to more complicated and I 
> have no experience in this area using a rebase workflow. Exchanging patches 
> sound like as good as an approach as any.

OK. I have updated README.developers accordingly (commit fb1dcd5).

@Hazen and Phil:

Does the language in this new paragraph seem clear enough about our
options for sharing experimental topic branch work that we are not going to
push to origin master immediately?




Alan