Move jemalloc'ed strdup/strndup definitions

RESOLVED FIXED in mozilla14

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 607179 [details] [diff] [review]
Move jemalloc'ed strdup/strndup definitions

Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=9363f5544207
Attachment #607179 - Flags: review?(khuey)
(Assignee)

Updated

5 years ago
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+
(Assignee)

Comment 3

5 years ago
(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?
(Assignee)

Comment 5

5 years ago
Created attachment 610104 [details] [diff] [review]
Move jemalloc'ed strdup/strndup definitions
Attachment #610104 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #607179 - Attachment is obsolete: true
(Assignee)

Comment 6

5 years ago
Created attachment 610105 [details] [diff] [review]
Move jemalloc'ed strdup/strndup definitions
Attachment #610105 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #610104 - Attachment is obsolete: true
Attachment #610104 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 7

5 years ago
Created attachment 610108 [details] [diff] [review]
Move jemalloc'ed strdup/strndup definitions
Attachment #610108 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
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+
(Assignee)

Comment 9

5 years ago
(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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.