[illumos-Developer] webrev: printf

Garrett D'Amore garrett at damore.org
Thu Oct 14 12:25:28 PDT 2010


On Thu, 2010-10-14 at 21:17 +0200, Joerg Schilling wrote:
> Guido Berhoerster <guido+illumos.org at berhoerster.name> wrote:
> 
> > Not only that but the code Garrett just committed contains a null
> > pointer dereference in case no arguments are passed to printf.
> 
> Something that shoule be detected by lint. Does this happen?

No, it had passed lint.

> 
> > If you just go ahead and commit stuff while there is still a
> > discussion going on where people (which do not belong to some
> > anonymous ksh93 fanboy group) voice legitimate concerns then I
> > wonder what the point of doing code reviews is?
> > A real code review would probably have caught trivial errors like
> > the above.
> 
> My impression is that there have been codereviwes for Illumos that were 
> not done by people who fully understand the related code. I would not only like 
> to see code reviews but wualified reviews.

What kinds of qualifications would you like?

I refused to subject all integrations to some committee of known ksh93
bigots.

My changes here were simple enough to understand, *and* I got the review
and approval of several different talented engineers.

The item above was just a mistake.  Sometimes mistakes make it through
code review.  Shit happens.  Get over it.

In this case, it was found early, and I've integrated the fix for it
already.

Can we *please* move on?

Btw, I will *remind* folks.  Prior to this the implementation of printf
we had was *CLOSED SOURCE*.  So, some bustage occurs in the process of
opening it up.  At *least* know we can look at the code.

If you don't like what I'm doing, feel free to fork.  At least thanks to
my extensive efforts you can *do* that now.  It was not all that long
ago that forking was not a practical option.

	- Garrett





More information about the Developer mailing list