Closed Bug 736963 Opened 12 years ago Closed 12 years ago

Move jemalloc'ed strdup/strndup definitions

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla14

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file, 3 obsolete files)

I want us to stop adding stuff directly in jemalloc code, which will allow easier jemalloc upgrades in the future. This bug is about strdup/strndup.
Attachment #607179 - Flags: review?(khuey) → review?(justin.lebar+bug)
Comment on attachment 607179 [details] [diff] [review]
Move jemalloc'ed strdup/strndup definitions

I have no idea if the build changes are right, but moving the definitions is fine.  I don't know if you want to get a review on the build changes from khuey.

Could we name the new file something other than jemalloc.c?
Attachment #607179 - Flags: review?(justin.lebar+bug) → review+
(In reply to Justin Lebar [:jlebar] from comment #2)
> Could we name the new file something other than jemalloc.c?

Any suggestions?

I'll probably rename memory/src to memory/build, too.
(In reply to Mike Hommey [:glandium] from comment #3)
> (In reply to Justin Lebar [:jlebar] from comment #2)
> > Could we name the new file something other than jemalloc.c?
> 
> Any suggestions?

extraMallocFuns.c?
Attachment #610104 - Flags: review?(ted.mielczarek)
Attachment #607179 - Attachment is obsolete: true
Attachment #610105 - Flags: review?(ted.mielczarek)
Attachment #610104 - Attachment is obsolete: true
Attachment #610104 - Flags: review?(ted.mielczarek)
Attachment #610108 - Flags: review?(ted.mielczarek)
Attachment #610105 - Attachment is obsolete: true
Attachment #610105 - Flags: review?(ted.mielczarek)
Comment on attachment 610108 [details] [diff] [review]
Move jemalloc'ed strdup/strndup definitions

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

::: Makefile.in
@@ +79,5 @@
>  endif
>  
>  ifdef MOZ_MEMORY
>  tier_base_dirs += memory/jemalloc
> +tier_base_dirs += memory/build

Just put these both in one assignment with a continuation.

::: mozglue/build/Makefile.in
@@ +48,5 @@
>  DIST_INSTALL = 1
>  
>  ifdef MOZ_MEMORY
> +SHARED_LIBRARY_LIBS = $(call EXPAND_LIBNAME_PATH,memory,$(DEPTH)/memory/build)
> +SHARED_LIBRARY_LIBS += $(call EXPAND_LIBNAME_PATH,jemalloc,$(DEPTH)/memory/jemalloc)

Single assignment with contuation here too.
Attachment #610108 - Flags: review?(ted.mielczarek) → review+
(In reply to Ted Mielczarek [:ted] from comment #8)
> Comment on attachment 610108 [details] [diff] [review]
> Move jemalloc'ed strdup/strndup definitions
> 
> Review of attachment 610108 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: Makefile.in
> @@ +79,5 @@
> >  endif
> >  
> >  ifdef MOZ_MEMORY
> >  tier_base_dirs += memory/jemalloc
> > +tier_base_dirs += memory/build
> 
> Just put these both in one assignment with a continuation.
> 
> ::: mozglue/build/Makefile.in
> @@ +48,5 @@
> >  DIST_INSTALL = 1
> >  
> >  ifdef MOZ_MEMORY
> > +SHARED_LIBRARY_LIBS = $(call EXPAND_LIBNAME_PATH,memory,$(DEPTH)/memory/build)
> > +SHARED_LIBRARY_LIBS += $(call EXPAND_LIBNAME_PATH,jemalloc,$(DEPTH)/memory/jemalloc)
> 
> Single assignment with contuation here too.

I did neither when landing, because in both cases, we're soon going to have ifdef/endif between both lines.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2a9fb626038
https://hg.mozilla.org/mozilla-central/rev/c2a9fb626038
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.