RE: [PATCH] STDCXX-563

[Scott Zhong <Sc...@roguewave.com>]

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:msebor@gmail.com] On Behalf Of Martin Sebor
> Sent: Wednesday, February 20, 2008 10:31 PM
> To: dev@stdcxx.apache.org
> 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

RE: [PATCH] STDCXX-563

[Farid Zaripov <Fa...@epam.com>]

> -----Original Message-----
> From: Scott Zhong [mailto:Scott.Zhong@roguewave.com] 
> Sent: Thursday, March 06, 2008 1:48 AM
> To: dev@stdcxx.apache.org
> Subject: RE: [PATCH] STDCXX-563
> 
> 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

  I have updated your patch and attached it to STDCXX-563 issue in JIRA.

  I have renamed _mutex_%platform%.h files to _atomic_%platform%.h;
extracted the generic functions for long and long long types to new
_atomic_generic.h file; extracted the template functions implemented
with using the mutex object to _atomic_mutex.h (perhaps this file should
be renamed to anoter name?) and created the header file _atomic.h to
dispatch #including the _atomic_xxx .hfiles.

 Also I have extracted the all OS-specific mutex things to corresponding
_mutex_%os_group%.h files.

Farid.