I had made modifications with the split according to hardware architecture.
http://www.fileden.com/files/2008/1/30/1729926/patch.stdcxx-563.v2.tar > -----Original Message----- > From: Martin Sebor [mailto:[EMAIL PROTECTED] On Behalf Of Martin Sebor > Sent: Wednesday, February 20, 2008 10:31 PM > To: [email protected] > Subject: Re: [PATCH] STDCXX-563 > > Scott Zhong wrote: > > Split _mutex.h into > > > > _mutex-aix.h > > _mutex-deccxx.h > > _mutex.h > > _mutex-i386gcc.h > > _mutex-ia64-x86-64.h > > _mutex-parisc.h > > _mutex-sgi.h > > _mutex-sparc.h > > > > http://www.fileden.com/files/2008/1/30/1729926/patch.stdcxx-563.tar > > > > Clearing the decks before my trip next week, I'm finally looking > at this patch... > > My first concern stems from the names of the new files: it seems > they reflect the partitioning of the current _mutex.h, which is > in some cases done for compilers and the intrinsics they provide, > and in other cases for hardware architectures and/or operating > systems. I hadn't realized that this was done so inconsistently > and was hoping for a cleaner split. I don't think we should have > _mutex-sgi.h right along _mutex-sparc.h, and certainly not > _mutex-i386gcc.h. We could achieve the same effect while at the > same time employing a more cohesive source organization under > a consistent naming convention by splitting the file by, say, the > OS, or the hardware architecture, or both, and including one from > the other as needed. > > I would expect _mutex.h to be a a thin "dispatch" kind of a header > with the only task to #include the header appropriate for the > platform (either compiler, or OS, or hardware architecture, but > not a mixture of all three). Such a header wouldn't contain any > platform-specific code (not even the Windows intrinsics). > > My other concern has to do with the very top of some of the new > files, such as _mutex-aix.h: > > } // namespace __rw > > #include <sys/atomic_op.h> > > _RWSTD_NAMESPACE (__rw) { > > First, the guard is missing, but it also looks like the opening > brace must be in _mutex.h? I don't think this is allowed by the > language, but even if it was, we wouldn't want to do that. It's > just not how source files should be structured. See, for example > http://accu.org/index.php/journals/473 > > I suggest you look at how the rw/_config.h header has been split > up (http://svn.apache.org/viewvc?view=rev&revision=382600) and > model the same approach in _mutex.h. If splitting it by compiler > doesn't make sense, maybe doing it by OS will. Or by hardware > architecture. Or even all three (although I'd try hard to keep > the number of new files to some reasonable minimum). > > Martin
