This is the mail archive of the mailing list for the libstdc++ project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] basic_filebuf: 45628 + non-modal I/O

On Sep 23, 2010, at 7:55 AM, Paolo Carlini wrote:

> Hi,
> two quick comments:
> - If, while working on these issues, you figure out some rather uncontroversial changes, can you please split them out to separate patches? (eg, I noticed your comment about imbue, I'm sure can be improved, at some point started growing rather wildly to quickly answer a series of PRs submitted by Petur)

OK. I made a conscious decision to produce a monolithic patch, given the outcome of the last round.

The observable changes to imbue are serious, and I should have listed them before:
1. Throw bad_cast or ios::failure instead of nulling out _M_codecvt.
2. Tolerate (encoding == -1) as doesn't really apply to us, per comment.
3. Reposition file when reading; do not adjust and preserve ext_buf.
4. If no file is open, defer all effects until open() is called.

#1 is the only thing that might cause a real issue. As noted, however, the user will already observe
bad_casts without my change… they will just be thrown at a later, perhaps unpredictable, time.
#2 is also an observable change, but is simply the removal of a bogus diagnostic.
#3 is a very subtle change and #4 is only "observable" in that it restores the original tolerance to #1.

I suppose that changing imbue and eliminating __check_facet could be considered
uncontroversial… however all those change sites are also modified to support the new caching.
So splitting off that diff would be some extra effort.

So, if there's a chance we'll commit it, I'd keep the current progress monolithic. The next thing on
the list, for overflow to preserve any remaining put area, is definitely practical separately, though.

> - Remember to *always* say on which machine you tested! I didn't know you are developing on Darwin and there we don't support named locales, thus testing is much weaker!

Yeah, it's pretty annoying that I can't access the locales on my machine through C++.
I should've said something about that when I noticed a test using Swedish by name.

The only target I'm using is x86_64-apple-darwin10.3.0. I really don't have the processor cycles
to test any more than I am. (And incremental builds would be very nice!)

	- D

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]