Re: CVS commit: src/lib/libc/stdio

[Masao Uebayashi <uebayasi%tombi.co.jp@localhost>]

On Tue, Sep 28, 2010 at 08:28:04AM +0200, Alistair Crooks wrote:
> On Tue, Sep 28, 2010 at 11:06:39AM +0900, Masao Uebayashi wrote:
> > > Index: src/lib/libc/stdio/fmemopen.c
> > > diff -u src/lib/libc/stdio/fmemopen.c:1.3 
> > > src/lib/libc/stdio/fmemopen.c:1.4
> > > --- src/lib/libc/stdio/fmemopen.c:1.3     Sat Sep 25 14:00:30 2010
> > > +++ src/lib/libc/stdio/fmemopen.c Mon Sep 27 16:50:13 2010
> > > @@ -79,16 +79,18 @@
> > >   if (p->cur >= p->tail)
> > >           return 0;
> > >   s = p->cur;
> > > - t = p->tail - 1;
> > >   do {
> > > -         if (p->cur == t) {
> > > -                 if (*buf == '\0')
> > > -                         *p->cur++ = *buf++;
> > > +         if (p->cur == p->tail - 1) {
> > > +                 if (*buf == '\0') {
> > > +                         *p->cur++ = '\0';
> > > +                         goto ok;
> > > +                 }
> > >                   break;
> > >           }
> > >           *p->cur++ = *buf++;
> > >   } while (--nbytes > 0);
> > >   *p->cur = '\0';
> > > +ok:
> > >   if (p->cur > p->eob)
> > >           p->eob = p->cur;
> > >  
> > > 
> > 
> > Do we have any reason to NOT use more assertions in such a critical
> > library code path?
> 
> I'm not sure how fmemopen(3) qualifies as a "critical library code path",
> but, whatever.

I suppose all libc functions are critical.  No idea how others think.

> As for more and more assertions - if people remember the bind assertion
> fun we had in July 2009, then I'm sure there would be people asking for
> less assertions, not more, especially in library code.
> 
> By all means check and warn, but please do not cause execution to halt
> unless it's absolutely essential.

I meant _DIAGASSERT(), which is expanded to real assert() iff DIAGNOSTIC
is defined.  Assertions are to express code's intent.

Masao

> 
> Regards,
> Alistair

-- 
Masao Uebayashi / Tombi Inc. / Tel: +81-90-9141-4635