On Thu, May 15, 2008 at 1:54 AM, Tony Balinski <[EMAIL PROTECTED]> wrote:
> Quoting Tony Balinski <[EMAIL PROTECTED]>:
>
>> Since I find I am unable to add a patch to the sourceforge bug report
>> mentioned above (am I missing some privileges here?), I'm sending this
>> patch to the list for perusal.
>>
>> Share and enjoy!
Thanks Tony.
Some comments below:
> +static long numAllocatedSinceLastGC = 0;
> +static size_t sizeAllocatedSinceLastGC = 0;
No need to initialize to 0. static vars are automatically 0. But I
don't know whether this is an Nedit style convention.
> +
> +/* structure to link allocated strings */
> +typedef struct GCStringTag {
> + struct GCStringTag *next;
> + char buff[1]; /* buff[0] is the same as the inUse flag
> */
> +} GCString;
Long overdue and nicely done. These handcoded things where really ugly.
To get more clearity, maybe something like this works too:
union {
char inUse;
char buff[1];
} u;
Or:
#define strInUse buff[1]
and than
str->strInUse
> +static void dropContext(RestartData *context)
> +{
> + /* unlink it from the list of continuations */
> + Continuations = context->next;
> +
> + context->next->prev = context->prev;
> + context->prev->next = context->next;
> +
> + if (Continuations == context)
> + Continuations = 0;
Should be NULL.
> -/* Allocate a new string buffer of length chars */
> +/*
> +** Allocate a new string buffer of length chars
> +** length does not include terminating null
> +*/
This comment is wrong, length actually need to include the terminating null.
> /* Allocate a new string buffer of length chars, and copy in the string s */
> char *AllocStringNCpy(const char *s, int length)
> {
> char *p = AllocString(length + 1); /* add extra char for forced \0 */
> - if (!p)
> - return p;
> - if (!s)
> - s = "";
> - p[length] = '\0'; /* forced \0 */
> - return strncpy(p, s, length);
> + if (!p) {
As you said, this is wrong ;-)
> +/*
> +** Marks all strings and/or arrays accessible in all active context stacks
> +*/
> +static void garbageCollectMarkActiveData()
> +{
> + RestartData sentinel;
> + RestartData *ctx;
> +
> + linkContext(&sentinel);
Why haven't you uses a sentinal based linked list in the first place?
> - iteratorValPtr->tag = INT_TAG;
> + iteratorValPtr->tag = ARITER_TAG;
> if (arrayVal.tag != ARRAY_TAG) {
> return(execError("can't iterate non-array", NULL));
> }
>
> - iteratorValPtr->val.arrayPtr = arrayIterateFirst(&arrayVal);
> + iteratorValPtr->val.iter.array = arrayVal.val.arrayPtr;
> + iteratorValPtr->val.iter.ptr = arrayIterateFirst(&arrayVal);
I thought some time ago about the semantics of changes to the array
while iterating it.
The only sane solution was to unconditionally copy the array for the
iteration. Some other toughts?
Bert
--
NEdit Develop mailing list - [email protected]
http://www.nedit.org/mailman/listinfo/develop
