[PATCH] simplify expand_tcbtab/alloc_tcb

[Denys Vlasenko]

On Wed, 2011-06-22 at 02:37 +0400, Dmitry V. Levin wrote:
> On Tue, Jun 21, 2011 at 08:21:26PM +0200, Denys Vlasenko wrote:
> > Hi,
> > 
> > Sometime ago expand_tcbtab/alloc_tcb were modified
> > so that they never return failure (they will abort instead).
> > 
> > This opens up a possibility for further simplifications.
> > 
> > This patch is a simple one which gets rid of a few
> > intermediate variables, simplifies a few expressions,
> > and uses error_msg_and_die instead of more verbose
> > fprintf+cleanup+exit sequence.
> > 
> > In alloc_tcp, I use memset to clear entire new tcp.
> > This not only saves a few bytes of code, but lowers chances
> > of future bugs where some data "leaks out" into new tcb's
> > from old ones because we forgot to re-initialize it.
> > 
> > Patch should be fairly safe, but please review anyway.
> > -- 
> > vda
> > 
> > diff -d -urpN strace.6/strace.c strace.7/strace.c
> > --- strace.6/strace.c	2011-06-21 20:12:03.206604267 +0200
> > +++ strace.7/strace.c	2011-06-21 20:11:27.220751557 +0200
> > @@ -1252,21 +1252,14 @@ expand_tcbtab(void)
> >  	   callers have pointers and it would be a pain.
> >  	   So tcbtab is a table of pointers.  Since we never
> >  	   free the TCBs, we allocate a single chunk of many.  */
> > -	struct tcb **newtab = (struct tcb **)
> > -		realloc(tcbtab, 2 * tcbtabsize * sizeof tcbtab[0]);
> > -	struct tcb *newtcbs = (struct tcb *) calloc(tcbtabsize,
> > -						    sizeof *newtcbs);
> > -	int i;
> > -	if (newtab == NULL || newtcbs == NULL) {
> > -		fprintf(stderr, "%s: expand_tcbtab: out of memory\n",
> > -			progname);
> > -		cleanup();
> > -		exit(1);
> > -	}
> > -	for (i = tcbtabsize; i < 2 * tcbtabsize; ++i)
> > -		newtab[i] = &newtcbs[i - tcbtabsize];
> > +	int i = tcbtabsize;
> > +	struct tcb *newtcbs = calloc(tcbtabsize, sizeof newtcbs[0]);
> >  	tcbtabsize *= 2;
> > -	tcbtab = newtab;
> > +	tcbtab = realloc(tcbtab, tcbtabsize * sizeof tcbtab[0]);
> 
> No, this intermediate newtab is still necessary because cleanup() uses
> tcbtab.  For the same reason, tcbtabsize could be changed only when
> tcbtab is already updated, e.g.
> 	tcbtab = newtab;
> 	tcbtabsize = newtabsize;

Yes. This should be better:

+       int i = tcbtabsize;
+       struct tcb *newtcbs = calloc(tcbtabsize, sizeof newtcbs[0]);
+       struct tcb **newtab = realloc(tcbtab, tcbtabsize * 2 * sizeof tcbtab[0]);
+       if (newtab == NULL || newtcbs == NULL)
+               error_msg_and_die("expand_tcbtab: out of memory");
        tcbtabsize *= 2;
        tcbtab = newtab;
+       while (i < tcbtabsize)
+               tcbtab[i++] = newtcbs++;

-- 
vda