Closed Bug 481579 Opened 11 years ago Closed 11 years ago

changes to shunt and tools for jemalloc on windows ce

Categories

(Firefox Build System :: General, defect)

ARM
Windows CE
defect
Not set

Tracking

(fennec1.0a1-wm+)

RESOLVED FIXED
Tracking Status
fennec 1.0a1-wm+ ---

People

(Reporter: blassey, Assigned: blassey)

References

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 4 obsolete files)

Attached patch patch v.1 (obsolete) — Splinter Review
No description provided.
Attachment #365603 - Flags: review?(doug.turner)
Attachment #365603 - Flags: review?(ted.mielczarek)
Attachment #365603 - Attachment is patch: true
Attachment #365603 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 365603 [details] [diff] [review]
patch v.1

why:

+#include <wchar.h>

This forces wchar.h to be included by every file.

Can you pull all of the "C" memory functions into an 'extern "C" block' so that you only need to declare these functions once?
Attached patch patch v.2 (obsolete) — Splinter Review
Assignee: nobody → bugmail
Attachment #365603 - Attachment is obsolete: true
Attachment #365920 - Flags: review?(doug.turner)
Attachment #365603 - Flags: review?(ted.mielczarek)
Attachment #365603 - Flags: review?(doug.turner)
Attachment #365920 - Flags: review?(ted.mielczarek)
Attachment #365920 - Flags: review?(ted.mielczarek) → review?(benjamin)
Attached patch patch v.3 (obsolete) — Splinter Review
this gets rid of the reference to mozce_coredll
Attachment #365920 - Attachment is obsolete: true
Attachment #366759 - Flags: review?(doug.turner)
Attachment #365920 - Flags: review?(doug.turner)
Attachment #365920 - Flags: review?(benjamin)
Attachment #366759 - Flags: review?(benjamin)
Comment on attachment 366759 [details] [diff] [review]
patch v.3

look okay.  mostly nits.

ooc, what is this change for:

+#define _CRTIMP __declspec(dllexport)

nit:  
ce_wcsndup( const unsigned

No space between the ( and the type.

+ * Portions created by the Initial Developer are Copyright (C) 2008

2009

+mozce_strndup( const char *src, size_t len ) {

check for malloc failure.

same thing for the rest of these functions.


-static bool env_ready = false;
+static bool env_ready = FALSE;

ooc, was this needed?


-  args[i++] = "/Zc:wchar_t-";          //

Why?


   args[i++] = LIB_PATH;
 
   argpath_conv(&argv[1], &args[i]);
-
+  

and

-
+  args[i++] = "/NODEFAULTLIB";
+    
+#ifdef MOZ_MEMORY

kill the whitespace.
Attachment #366759 - Flags: review?(doug.turner) → review-
(In reply to comment #4)
> (From update of attachment 366759 [details] [diff] [review])
> look okay.  mostly nits.
> 
> ooc, what is this change for:
> 
> +#define _CRTIMP __declspec(dllexport)

That is so when we include stdlib, it defines malloc, free etc. with the same linkage as we're using (otherwise its dllimport, which causes and error)


> -  args[i++] = "/Zc:wchar_t-";          //
> 
> Why?

/Zc:whcar_t- is in CFLAGS, defining it here is redundant and confusing if try to you change it
tracking-fennec: --- → 1.0a1-wm+
Status: NEW → ASSIGNED
Attached patch patch v.4 (obsolete) — Splinter Review
Attachment #366759 - Attachment is obsolete: true
Attachment #368880 - Flags: review?(doug.turner)
Attachment #366759 - Flags: review?(benjamin)
Attachment #368880 - Attachment is obsolete: true
Attachment #368888 - Flags: review?(doug.turner)
Attachment #368880 - Flags: review?(doug.turner)
Attachment #368888 - Flags: review?(doug.turner) → review+
pushed http://hg.mozilla.org/mozilla-central/rev/4054dd5d4084
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Attachment #368888 - Flags: approval1.9.1?
Comment on attachment 368888 [details] [diff] [review]
patch v.5, quiets compiler warnings

a191=beltzner
Attachment #368888 - Flags: approval1.9.1? → approval1.9.1+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.