[illumos-Developer] webrev: reimplementation of od

[Gordon Ross]

On Tue, Oct 19, 2010 at 12:00 AM, Garrett D'Amore <garrett at damore.org> wrote:
>
> In order to answer some feedback questions, and address some bugs that
> were pointed out in review, I've changed the implementation somewhat.
>
> The new webrev is here: http://pastebin.ca/1966514
>
> Thea diffs from the previous (yesterday's) version of the code are here:
> http://pastebin.ca/1966521
>
> The new logic should be simpler to read, since it just uses a circular
> FIFO instead of three separate buffers.  It uses power of two math for
> efficiency.
> [...]

Lines 67-71: I'd prefer to see comments on what these fields mean.
I figured it out, but it would make it easier to read. i.e.

/*
 * this structure describes our ring buffer...
 * always power-of-two length ... etc.
 */
        int     prod;        /* producer index */
        int     cons;        /* consumer index */
        int     mask;        /* to constrain indexes. */
        int     size;        /* XXX - is this used in the program? */
Or does the code always use mask+1 instead?
Suggest using size in place of mask+1, or just
get rid of size and use mask+1 in its place.

Otherwise looks OK to me by inspection.
I have not tested it.

Gordon