Re: Re: Big leak fished!

So there was one more moto_strdup in motoi that was being used to 
create MotoVar names. This one was in motoi_dereference_lval. It looks 
like I can also safely change it from

	/* Get the name of the member variable to dereference to from operand 
1 */
	varn = moto_strdup(env, uc_str(p, 1));

to

	/* Get the name of the member variable to dereference to from operand 
1 */
	varn = uc_str(p, 1);

By making these changes I think we avoid the varnames hashtable and 
associated functions you propose in your patch. Is that right ?

-Dave

On Sunday, August 3, 2003, at 03:57  PM, David Hakim wrote:

>
> On Thursday, July 31, 2003, at 09:12  AM, Stefano Corsi wrote:
>
>> Hi Dave,
>>
>> I was not conformatble with moto dying in this kind of loops:
>>
>> ${
>>
>>         int i = 10000;
>>         while (i-- > 0) {
>>                 int j = 10000;
>>                 while(j-- > 0) {
>>                         int k;
>>                 }
>>         }
>>
>> }$
>>
>> so I noticed that if you take away the declaration and you have only 
>> this:
>>
>> ${
>>
>>         int i = 10000;
>>         while (i-- > 0) {
>>                 int j = 10000;
>>                 while(j-- > 0) {
>>                 }
>>         }
>>
>> }$
>>
>> moto does not die.
>> So the problem is that when moto_freeFrame frees all the frame 
>> variables, it
>> does not free the strdupped string representing the name and this can 
>> lead to
>> megabytes of shared memory, in case of such loops (even if probably 
>> they are
>> not so common nor useful...):
>>
>> if(env->mode != COMPILER_MODE) {
>> 	moto_freeVal(env,var->vs);
>> } else {
>> 	opool_release(env->valpool,var->vs);
>> }
>> free(var->n); // <-- this was missing
>> free(var);
>>
>
> I believe the above variable name free was left out on-purpose for the 
> cases when class member variables were pushed onto the frame. I notice 
> specifically in motoi_callMDF we have:
>
> 		e = vec_elements(mcd->memberVarNames);
> 		while(enum_hasNext(e)){
> 			char* varn=(char*)enum_next(e);
> 			
> 			/* Fake variable shadowing by simply not loading in a member var
> 				with the same name as an argument */
> 				
> 			if(moto_getFrameVar(env,varn) == NULL) {
> 				MotoVar* var = moto_createVar(
> 					env,varn,
> 					mcd_getMemberType(mcd,varn),
> 					0,
> 					'\1',
> 					mci+mcd_getMemberOffset(mcd,varn)
> 				);
> 				stab_put(env->frame->symtab, var->n, var);
> 			}
> 		}
> 		enum_free(e);
>
> which definitely does not duplicate the variable name. Also in 
> motoi_callMDF we don't duplicate the argument names when calling moto 
> defined functions:
>
> 	/* Push arguments onto the frame by declaring them */
> 	for(i=0;i<argc;i++){
> 		UnionCell* argdec = uc_operand(argListUC,i);
> 		UnionCell* atype_uc = uc_operand(argdec,0);
> 		
> 		int argdecDim ;
> 		
> 		char *aname;
> 		MotoVar* avar;
> 		MotoType* atype;
> 		
> 		atype = motox_extractType(atype_uc);
> 		aname = uc_str(argdec,1);
> 		argdecDim = uc_opcount(uc_operand(argdec, 2));
>
> 		/* Declare the argument variable */
> 		avar = moto_declareVar(env, aname, atype->name, argdecDim, '\0');
>
> 		moto_setVarVal(env, avar, args[i]);
> 	}
>
> So I'm thinking that calling moto_strdup on the variable name in 
> motoi_declare is not necessary (since variable names aren't 
> specifically freed until the end of a page anyhow). I'm going to try 
> removing it from motoi_declare by changing
>
> 		/* Get the variable name and dimension */
> 		varn = moto_strdup(env, uc_str(declarator_uc, 0));
> 		vdim = uc_opcount(uc_operand(declarator_uc, 1));
>
> to
>
> 		/* Get the variable name and dimension */
> 		varn = uc_str(declarator_uc, 0);
> 		vdim = uc_opcount(uc_operand(declarator_uc, 1));
>
> and see what breaks :)
>
> -Dave
>
> BTW: after making this change all tests still pass
>
>> Now it works. Slowly, but it works!
>>
>> Stefano
>>
>>
>
>