Re: [PATCH] STDCXX-522

[Everton Araujo]

Sorry Martin, I didn't observe the coding style ... my bad. I'll change it
right now.
About the test, I think it's good for avoiding unnecessary xsputn calls
considering that it just exists when char count (__nchar) is equals 0.
May I suggest something? (If not, jmp _end) I think you would consider
transform xsputn in an inline function which may call "_xsputn" (the full
implementation) just when count greater than 0, automatically avoiding
unnecessary calls. What do you think about it? Please remember, it's just a
suggestion, don't hate me, give me a smile :-)
_end:
The test is not required for correct patch behavior. You may ignore it with
no regards. :-)

Regards.

Everton.

2007/8/18, Martin Sebor <[EMAIL PROTECTED]>:
>
> Everton Araujo wrote:
> > Thank you Martin and Andrew for helping me.
> >
> > Below is the patch for STDCXX-522 (std::filebuf::overflow(EOF) writes
> EOF to
> > file in unbuffered mode)
>
> Thanks for the patch! I have a couple of questions for you but first
> let me make a general comment about our formatting style. The stdcxx
> style guidelines haven't been migrated from Rogue Wave to Apache yet,
> so until we have migrated them, contributors like you need to try to
> figure them out by observing the existing code they are patching.
> A couple of the basic ones are:
>
>    1. Use 4-space indents (no TABs).
>    2. Separate every opening parenthesis, bracket, or curly brace
>       from the preceding symbol by a single space.
>
> So by the way of example, ...
>
> >
> > Index: include/fstream.cc
> > ===================================================================
> > --- include/fstream.cc    (revision 566470)
> > +++ include/fstream.cc    (working copy)
> > @@ -351,8 +351,15 @@
> >          _RWSTD_STREAMSIZE __nchars;
> >
> >          if (__unbuf) {
> > +          if(this->_C_is_eof(__c)){
>
> ...this should be (note the two additional spaces before the if
> and the one space after the if, after _C_is_eof, and before the
> open curly brace):
>
>    +            if (this->_C_is_eof (__c)) {
>
> Now for the questions...
>
> > +            _C_cur_pos.state (0);
> > +            __buf   = 0;
> > +            __nchars = 0;
> > +          }
> > +          else{
> >              __buf    = &__c_to_char;
> >              __nchars = 1;
> > +          }
> >          }
> >          else {
> >              // call xsputn() with a special value to have it flush
> > @@ -364,7 +371,7 @@
> >          // typedef helps HP aCC 3.27
> >          typedef basic_filebuf _FileBuf;
> >
> > -        if (__nchars != _FileBuf::xsputn (__buf, __nchars))
> > +        if (__nchars && __nchars != _FileBuf::xsputn (__buf, __nchars))
>
> Why is the extra test here necessary? I.f., why wouldn't the original
> code be sufficient?
>
> >              return traits_type::eof ();  // error while writing
> >      }
> >
> > @@ -424,7 +431,7 @@
> >          typedef basic_filebuf _FileBuf;
> >
> >          // return -1 on error to flush the controlled sequence
> > -        if (__nwrite != _FileBuf::xsputn (__special, __nwrite))
> > +        if (__nwrite && __nwrite != _FileBuf::xsputn (__special,
> __nwrite))
>
> Why is this change necessary at all?
>
> Martin
>