Last Comment Bug 826779 - Get DMD to build on Windows
: Get DMD to build on Windows
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DMD (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla20
Assigned To: :Ehsan Akhgari
:
Mentors:
Depends on:
Blocks: 819839
  Show dependency treegraph
 
Reported: 2013-01-04 10:55 PST by :Ehsan Akhgari
Modified: 2013-01-08 20:51 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (3.43 KB, patch)
2013-01-04 10:58 PST, :Ehsan Akhgari
netzen: review+
Details | Diff | Splinter Review

Description :Ehsan Akhgari 2013-01-04 10:55:04 PST
I have a patch which makes Windows build successfully with --enable-dmd.  I'd like to land it for now, but I haven't tested DMD yet.  I'll do that work in bug 819839.
Comment 1 :Ehsan Akhgari 2013-01-04 10:58:36 PST
Created attachment 697994 [details] [diff] [review]
Patch (v1)
Comment 2 Brian R. Bondy [:bbondy] 2013-01-04 11:41:19 PST
Comment on attachment 697994 [details] [diff] [review]
Patch (v1)

Review of attachment 697994 [details] [diff] [review]:
-----------------------------------------------------------------

::: memory/replace/dmd/DMD.cpp
@@ +49,5 @@
> +  return si.dwPageSize;
> +}
> +static void* valloc(size_t size)
> +{
> +  return VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);

> To reserve and commit pages in one step, call VirtualAlloc 
> with MEM_COMMIT | MEM_RESERVE.
>
> The function fails if you attempt to commit a page that has not been 
> reserved. The resulting error code is ERROR_INVALID_ADDRESS.

This MSDN info doc is a bit confusing to me.
I think reserving only matters for rounding though if you have the first parameter non null.  So what you have is fine as far as I can tell.

I wouldn't be opposed to passing in MEM_RESERVE | MEM_COMMIT
Comment 3 Nicholas Nethercote [:njn] 2013-01-04 12:35:05 PST
Comment on attachment 697994 [details] [diff] [review]
Patch (v1)

Review of attachment 697994 [details] [diff] [review]:
-----------------------------------------------------------------

::: memory/replace/dmd/DMD.cpp
@@ +47,5 @@
> +  SYSTEM_INFO si;
> +  GetSystemInfo(&si);
> +  return si.dwPageSize;
> +}
> +static void* valloc(size_t size)

Why have you defined valloc()?
Comment 4 :Ehsan Akhgari 2013-01-04 13:11:22 PST
(In reply to Brian R. Bondy [:bbondy] from comment #2)
> Comment on attachment 697994 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 697994 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: memory/replace/dmd/DMD.cpp
> @@ +49,5 @@
> > +  return si.dwPageSize;
> > +}
> > +static void* valloc(size_t size)
> > +{
> > +  return VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
> 
> > To reserve and commit pages in one step, call VirtualAlloc 
> > with MEM_COMMIT | MEM_RESERVE.
> >
> > The function fails if you attempt to commit a page that has not been 
> > reserved. The resulting error code is ERROR_INVALID_ADDRESS.
> 
> This MSDN info doc is a bit confusing to me.
> I think reserving only matters for rounding though if you have the first
> parameter non null.  So what you have is fine as far as I can tell.
> 
> I wouldn't be opposed to passing in MEM_RESERVE | MEM_COMMIT

OK, I'll do that, but I don't think that makes any difference.

(In reply to Nicholas Nethercote [:njn] from comment #3)
> Comment on attachment 697994 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 697994 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: memory/replace/dmd/DMD.cpp
> @@ +47,5 @@
> > +  SYSTEM_INFO si;
> > +  GetSystemInfo(&si);
> > +  return si.dwPageSize;
> > +}
> > +static void* valloc(size_t size)
> 
> Why have you defined valloc()?

Because the Microsoft CRT does not support valloc, which is later on called in this file.
Comment 6 Phil Ringnalda (:philor) 2013-01-05 16:29:31 PST
https://hg.mozilla.org/mozilla-central/rev/2c7233ca6ffd

Note You need to log in before you can comment on or make changes to this bug.