Closed Bug 581478 Opened 10 years ago Closed 7 years ago
warning: "strdup" redefined
In file included from ../../../dist/include/nscore.h:51, from /home/t_mattjo/src/firefox/mobilebase/widget/src/gtkxtbin/gtk2xtbin.h:48, from /home/t_mattjo/src/firefox/mobilebase/widget/src/gtkxtbin/gtk2xtbin.c:46: ../../../dist/include/mozilla/mozalloc_macro_wrappers.h:60:1: warning: "strdup" redefined In file included from /usr/include/string.h:635, from ../../../dist/system_wrappers/string.h:3, from /usr/include/X11/Intrinsic.h:64, from ../../../dist/system_wrappers/X11/Intrinsic.h:3, from /home/t_mattjo/src/firefox/mobilebase/widget/src/gtkxtbin/gtk2xtbin.h:44, from /home/t_mattjo/src/firefox/mobilebase/widget/src/gtkxtbin/gtk2xtbin.c:46: /usr/include/bits/string2.h:1316:1: warning: this is the location of the previous definition
Looks like mozalloc_macro_wrappers.h does nothing but insert another function call on the stack since bug 738176 landed and it didn't do much more before that. The plan as of 2010 to make these use the infallible allocators hasn't been actioned, so it seems about time to remove this and mozalloc_undef_macro_wrappers.h.
I haven't hit this locally, but here's what I have in /usr/include/bits/string2.h at around line 1316, on my local machine: > # define __strdup(s) \ [...SNIP...] > # if defined __USE_SVID || defined __USE_BSD || defined __USE_XOPEN_EXTENDED > # define strdup(s) __strdup (s) > # endif > # endif so string2.h does indeed have a #define for strdup, and we're redefining it without ever #undef'ing it. Disregarding comment 1 for the moment: a trivial band-aid for this (to allow bug 896292 to land) would be to just add an #ifdef / #undef before we (re)define strdup.
Per comment 2, this just #undef's strdup before we #define it, if it's already defined. This should be functionally equivalent to what we're already doing, aside from the fact that it won't spam a warning.
Glandium had concerns over this approach on IRC last night, IIRC.
Comment on attachment 781360 [details] [diff] [review] band-aid Tagging him for feedback, then... I'm not a macro wizard (my CS program shunned them as evil), but I don't immediately see why anything would be wrong with this.
Attachment #781360 - Flags: feedback?(mh+mozilla)
My only concern is that it would only fix it in the cases where mozalloc_macro_wrappers.h is included after whatever includes /usr/include/bits/string2.h. But that's probably good enough.
Comment on attachment 781360 [details] [diff] [review] band-aid Review of attachment 781360 [details] [diff] [review]: ----------------------------------------------------------------- ::: memory/mozalloc/mozalloc_macro_wrappers.h @@ +27,5 @@ > +/* > + * Note: on some platforms, strdup may be a macro instead of a function. > + * In that case, we have to #undef it to avoid warnings about redefining it. > + */ > +#ifdef strdup You don't strictly need the ifdef.
Attachment #781360 - Flags: feedback?(mh+mozilla) → feedback+
Comment on attachment 781360 [details] [diff] [review] band-aid Let's make this a r+.
I removed the #ifdef, per glandium's suggestion, and pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/924c4d59db8b karl, I don't know enough about mozmemory to respond intelligently to comment 1, but if there's anything we should do to address your points there, perhaps that can happen in a new bug?
Comment 1 was mostly trying to say that we are wasting our time maintaining a header that does nothing beneficial, so the header should be removed. The only problem with this bandaid, beyond comment 6, is that mozalloc_undef_macro_wrappers.h doesn't revert what mozalloc_macro_wrappers.h does, but mozalloc_undef_macro_wrappers.h isn't used anywhere critical anyway. I don't have any objection to landing the bandaid.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.