[PATCH] Avoid some memory waste while allocating tcbtab.

Nahim El Atmani nahim+dev at naam.me
Tue Mar 1 16:44:57 UTC 2016


Hello,

On Tue, 01 Mar 2016 02:27:09 +0300, Dmitry V. Levin wrote:
> [...]
> > +	if (!tcbtabsize) {
> > +		create_tcbtab();
> > +	} else {
> > +		i = tcbtabsize;
> > +		newtcbs = xcalloc(tcbtabsize, sizeof(newtcbs[0]));
> > +		newtab = xreallocarray(tcbtab, tcbtabsize * 2,
> > +				       sizeof(tcbtab[0]));
> > +		tcbtabsize *= 2;
> > +		tcbtab = newtab;
> > +		while (i < tcbtabsize)
> > +			tcbtab[i++] = newtcbs++;
> > +	}
> >  }
> 
> This is better.  Do you think create_tcbtab that's called just once
> from expand_tcbtab worth to be a separate function?

I mainly did that for visibility and testing purposes at first (since it's
easier to test small atomic chunks of code than a big fully featured function).
I agree, however, that there is no advantages of spliting the code in this
case since the code is covered in both situations. Fixed in the next version.

> 
> What if these two branches of code were merged, e.g.
> 
> 	unsigned int new_tcbtabsize, alloc_tcbtabsize;
> 	if (tcbtabsize) {
> 		 alloc_tcbtabsize = tcbtabsize;
> 		 new_tcbtabsize = tcbtabsize << 1;
> 	} else {
> 		alloc_tcbtabsize = new_tcbtabsize = 1;
> 	}
> 	etc.
> 
> Wouldn't the result code be more compact and somewhat more readable?

I would say that it's more compact for sure, but I still have some doubts
concerning the readability since it introduces some less obvious behaviors. As
an illustration let's take the complete example:

>	unsigned int i, new_tcbtabsize, alloc_tcbtabsize;
>	struct tcb *newtcbs;
>	struct tcb **newtab;
>
>	if (tcbtabsize) {
>		alloc_tcbtabsize = tcbtabsize;
>		new_tcbtabsize = tcbtabsize * 2;
>	} else {
>		new_tcbtabsize = alloc_tcbtabsize = 1;
>	}
>
>	i = tcbtabsize;
>	newtcbs = xcalloc(alloc_tcbtabsize, sizeof(newtcbs[0]));
>	newtab = xreallocarray(tcbtab, new_tcbtabsize, sizeof(tcbtab[0]));

Here xreallocarray() is used as a xmalloc() since tcbtab lives in the bss and
was not defined yet. Nothing wrong with that except that one have to get the
whole picture before understanding this code.
Nevertheless, I think this version is the right way to do it. I tried to get
the most readable version as possible, but it's far from being the most compact
one. I'm sending a new version.

Regards,

-- 
Nahim El Atmani <nahim+dev at naam.me>
http://naam.me/




More information about the Strace-devel mailing list