RE: commit into mysql-6.0-wtf:mysql-6.0-wtf branch (vvaintroub:2653) Bug#24509, WL#3049

[Vladislav Vaintroub]

Hi all,
what Davi describes is a sane approach, that  if consistently applied can
result into high quality library, like Apache Portable Runtime. Hence , it
is not applicable for us ;)
Seriously, we do need a top-level mapper functions. The reason is that many
mysys IO functions include large chunks of code shared on all OS (error
handling, messing up with my_file_info array) and  it does not make sense to
replicate those chunks for every OS. What could make sense for Windows is to
create win_file_io.c with ANSI/Posix wrappers. Then, use these wrappers in
the mysys function.

To illustrate, this approach would make piece of code like this 

#ifndef __WIN__
  fd = fdopen(Filedes, type);
#else
  {
    /* Convert OS file handle to CRT file descriptor and then call fdopen*/
    int crt_fd = _open_osfhandle((intptr_t)my_get_osfhandle(Filedes), 0);
    if(crt_fd < 0)
      fd = 0;
    else
      fd = fdopen(crt_fd, type);
  }
#endif

less cluttered (still with #ifdefs though)

#ifndef __WIN__
  fd = fdopen(Filedes, type);
#else
  fd = my_win_fdopen(Filedes, type);
#endif


Vlad
PS. Sorry to ignore Netware, I've absolutely clueless about it.


> -----Original Message-----
> From: Joerg Bruehe [mailto:joerg@stripped]
> Sent: Wednesday, June 04, 2008 11:00 AM
> To: Davi Arnaut
> Cc: Vladislav Vaintroub; commits@stripped
> Subject: Re: commit into mysql-6.0-wtf:mysql-6.0-wtf branch
> (vvaintroub:2653) Bug#24509, WL#3049
> 
> Hi Davi !
> 
> 
> Davi Arnaut wrote:
> > Hi Vlad,
> >
> > Vladislav Vaintroub wrote:
> >> #At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-6.0-wtf/
> >>
> >>  [[...]]
> >
> > Wouldn't this be a good opportunity to start to untangle the ifdefery
> > mess in mysys? We could start splitting the implementations into
> > different directories, for example:
> >
> > mysys/win32/file_io.c
> > mysys/unix/file_io.c
> > mysys/netware/file_io.c
> >
> > And each one would implement it's own specified version of my_foo.
> 
> I have a very strong feeling against such a split into separate files
> for platform-specific implementations.
> 
> 
> Sure, it would get rid of several "#if"/"#ifdef"/... and so make
> reading
> a bit easier.
> 
> 
> But it would also cause several source modules (files) which have to
> implement the same interface (for the caller), so any change to that
> interface would need to be done in several files in sync.
> 
> As long as these various implementations are in the same file, it is
> easy to search for the relevant identifier(s) using your editor, and
> find all occurrences.
> Hopefully, it occurs only once, and any differentiation occurs in its
> body - this guarantees an interface change is done in the same way for
> all implementations.
> 
> The moment you have multiple modules (by implementation) for one
> interface, keeping them in sync gets much more complicated - even more
> so, because your local testing will typically be on one platform only.
> 
> If you really feel different implementations should be split into
> different files, at least keep one common entry (mapper) module for
> them. (The drawback of this is of course reduced chances for the
> optimizer, as now the call crosses file boundaries.)
> 
> IMHO, a mapper function for the interface, with its body being just
> some
> "#if" around calls to different implementations, and these being
> defined
> as "static inline" functions (completely within large "#if" blocks) is
> as readable, and it allows for optimization.
> 
> 
> I have seen too many cases where developers changed one occurrence only
> (which they had tested) and forgot others (which "bk -r grep ..." would
> have immediately shown to them).
> 
> 
> Regards,
> Jörg
> 
> --
> Joerg Bruehe,  MySQL Build Team,  joerg@stripped   ([phone]
> Sun Microsystems GmbH,   Sonnenallee 1,   D-85551 Kirchheim-Heimstetten
> Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
> Vorsitzender des Aufsichtsrates: Martin Haering     Muenchen: HRB161028
> 
> 
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1