Merge lp:~jobinau/drizzle/mingwport into lp:~drizzle-trunk/drizzle/development

[Jobin Augustine]

First baby step towards windows port of drizzle client (command prompt).
this patch is to prevent build break.

[Monty Taylor (mordred)]

There are a couple of fixes that need to happen before this is merged, although I'm happy to see it working!

The #include "config.h" line needs to be removed from my_sys.h. We do not put that in headers - instead, it should be the first thing included by the .cc file. (Which should already be happening) If this is breaking, please let me know how and I'll help you find a solution.

Also, my_sys.h should really not be chmod +x - there is no need for it to be executable.

#ifdef HAVE_SYS_MMAN_H addition looks great though.

review: Needs Fixing

[Jobin Augustine (jobinau)]

Thanks Monty for this review.
I know, I need to fix these kind of stuffs to drizzle standard and thats why
trying to push small chunks.

On Sun, Aug 15, 2010 at 2:44 AM, Monty Taylor <email address hidden> wrote:

> Review: Needs Fixing
> There are a couple of fixes that need to happen before this is merged,
> although I'm happy to see it working!
>
> The #include "config.h" line needs to be removed from my_sys.h. We do not
> put that in headers - instead, it should be the first thing included by the
> .cc file. (Which should already be happening) If this is breaking, please
> let me know how and I'll help you find a solution.
>

I shall take a closer look, why it is required.
I think i may need a more guidance in this regard.

> Also, my_sys.h should really not be chmod +x - there is no need for it to
> be executable.

Oh my god. file copy from windows is messing up permissions. :). i will
correct.

> #ifdef HAVE_SYS_MMAN_H addition looks great though.
>

Thank you. but just wondering whether the definition of HAVE_SYS_MMAN_H
will come without "config.h"

--
> https://code.launchpad.net/~jobinau/drizzle/mingwport/+merge/32450<https://code.launchpad.net/%7Ejobinau/drizzle/mingwport/+merge/32450>
> You are the owner of lp:~jobinau/drizzle/mingwport.
>

Thank you,
Jobin.

[Jobin Augustine (jobinau)]

OK. a second thought put me in better understanding..my stupidity.
so the idea is we need to include config.h in .CC file before including <sys/mman.h>
i will modify accordingly and resubmit.

[lp:~jobinau/drizzle/mingwport]

1704. By jobin <jobin@jobin-laptop> on 2010-08-15

removed config.h
corrected the file permission

[Jobin Augustine (jobinau)]

Modified the file accordingly.
requesting your review and merge.

[Monty Taylor (mordred)]

Looks great! Thanks!

review: Approve

=== modified file 'drizzled/internal/my_sys.h'
--- drizzled/internal/my_sys.h	2010-06-01 21:54:27 +0000
+++ drizzled/internal/my_sys.h	2010-08-15 07:28:44 +0000
@@ -49,7 +49,9 @@
 
 #include <drizzled/dynamic_array.h>
 
+#ifdef HAVE_SYS_MMAN_H 
 #include <sys/mman.h>
+#endif
 
 #include "drizzled/qsort_cmp.h"