simulavr-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Simulavr-devel] Patches - again


From: Onno Kortmann
Subject: Re: [Simulavr-devel] Patches - again
Date: Sun, 22 Mar 2009 22:26:35 +0100
User-agent: KMail/1.9.5

Hi Joerg, Klaus and all the others,

> I'm afraid once you (or whoever) committed it to CVS, nobody is ever
> going to touch it again.  So my very personal opinion: it's OK to
> initially commit "development quality" code (basic requirement: it
> must compile for everybody), *but* only as long as you continue
> working onto it until it's got "good quality" (or even "excellent
> quality").
ACK. Although people tend to argue over 'good quality' a lot ;-)

> Otherwise, over time, the quality of the final product 
> will severely suffer.  Seriously, Onno, some of your ideas sound
> really cool, but then, some of your comments make me a little scary as
> well. ;-)  (Like the one explaining that there's still a memory leak
> within.)  For sure, I wouldn't want to have that within a final
> release myself.
I think you're talking about this piece here:

---
 /* We may leak a bity of memory for the pointer in the vector,
       but... what the hell! */
    delete devices[handle];
    devices[handle]=0;
---
Well, I am deliberately 'leaking' memory in the sense that I have this C++
vector of handles and if one AVR device gets removed from within the
verilog simulation, there will be useless '0' entry in this vector,
without the vector ever being reduced in size.

I could have used a fancy dynamic data structure, but I figured that 
creating/removing whole devices from a simulation is a rather rare act
which is only done upon startup/teardown of the simulation.
So I thought that it would be ok to mark it with this comment but otherwise
leave it as it is. Especially as there are other, more severe memory leaks in 
simulavr
(See the broken patch 0006 from me).

BTW, I'd like to point out that there is the old verilog patch from me
in upstream CVS, more or less useless at the moment and without any examples.
I updated this patch (which I considered 'developer quality' at the time
I submitted it)  and I think it could get more 'stable' as soon as others 
start to use and maybe like it.

This code apparently slipped in with the big 'the old stuff' commit
from Klaus just before all the recent activity on this list.

IIRC there were some valid complaints and questions about this old code which 
is in the CVS now,
which I tried to address in the current update. So I try to update my code
and make it better (...though it may take years for updates! :-)

I'm still working on an SPI example...


> As Klaus' response above sounded a little annoyed, and even partially
> like he might completely abandon this project (at least in public), I
> continued some private discussion with him.  That way, we could handle
> it in German, which was simply easier for the job.  After a couple of
> emails, his initial annoyance was basically gone, and he assured me
> that he primarily wants to step back a little, to allow others to
> eventually step in. 
Klaus, I did'nt want to bypass you; My motivation was mainly this part
in a mail from Eric Weddington:
'You could just put them on the patch tracker that way all of us can use them.'

I known damn well that you have been the guy who rewrote the whole thing
from (IMHO ugly, sorry :-) C to nice C++, as you sent me some preliminary 
.tar.gz files 
of your translation some years ago, just before you put them into CVS.

Best regards,

Onno




reply via email to

[Prev in Thread] Current Thread [Next in Thread]