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 <[email protected]>
http://naam.me/

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to