Re: bk commit into 5.0 tree (tsmith:1.2576) BUG#20748

[Sergei Golubchik]

Hi!

On Feb 26, Timothy Smith wrote:
> Hi, SerG.
> 
> Thanks for the extra review.
> 
> On Tue, Feb 26, 2008 at 02:16:23PM +0100, Sergei Golubchik wrote:
> > 1. Why did you need a init_default_directories() variable where you
> > store the actual function to be called ? Why not to do it like we do
> > elsewhere e.g.
> > 
> > static void init_default_directories()
> > {
> > #ifdef __NETWARE__
> > ...
> > #elif defined(__EMX__) || defined(OS2)
> > ...
> 
> Originally I had coded it like this, and TheK found it difficult to see,
> for a particular OS, what the actual behavior is.  So I separated it out
> into per-OS functions as you see it now, which more in the style of OO
> virtual functions.
> 
> Here's the preliminary patch which Kristofer objected to:
> http://lists.mysql.com/commits/35732
> 
> It was intended as a readability improvement.  Perhaps it backfired?

Hmm, indeed, I'd choose one function with ifdefs :(
 
> > 2. Why ADD_DIRECTORY is defined differently for debug and non-debug
> > builds ? Remember, the fewer ifdefs we have in the code, the better.
> 
> The sole reason for this was to avoid a warning for the unused return
> value variable rc, which is checked with DBUG_ASSERT() but cast to void
> in the non-debug version.
> 
> I can't put #ifdef inside a macro #define, so this was my solution.

I'd rather prefer __attribute__((unused)).
 
Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer/Server Architect
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140