Everton Araujo wrote:
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.
Perhaps it is. But if it's not necessary to fix the bug such
a change should be made in a separate patch to minimize the
scope of the bugfix. We try to highlight this guideline on
our bugs page but possibly not be as clearly as we should:
http://incubator.apache.org/stdcxx/bugs.html#patch_format
May I suggest something? (If not, jmp _end)
Yes! Please do! :) Suggestions for improvements are welcome!
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 :-)
I assume you're proposing to add a new inline function that
would be used internally by our implementation (we can't change
the name xsputn() since it's a required interface). That might
be something to consider, although there already is a public
inline function called sputn() that presumably could be used
for this *if* the standard requirements on it were relaxed so
as to allow it to return without calling xsputn() when its
second argument is 0. The other issue with calling sputn()
from overflow() is that it would end up calling the xsputn()
defined in a derived class, which is not permitted by the
standard (that's why the call to xsputn() is qualified with
the name of basic_filebuf). So this will need some thought.
Thanks again for your suggestion and the patch. I'll test it
and commit it as soon as I've seen successful test results.
Martin
_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
