Closed Bug 581478 Opened 9 years ago Closed 6 years ago

warning: "strdup" redefined

Categories

(Core :: XPCOM, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: jdm, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(Whiteboard: [build_warning])

Attachments

(1 file)

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
Whiteboard: [build_warning]
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.
Attached patch band-aidSplinter Review
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.
Attachment #781360 - Flags: review?(khuey)
Glandium had concerns over this approach on IRC last night, IIRC.
Flags: needinfo?(mh+mozilla)
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.
Flags: needinfo?(mh+mozilla)
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+
Assignee: nobody → dholbert
Comment on attachment 781360 [details] [diff] [review]
band-aid

Let's make this a r+.
Attachment #781360 - Flags: review?(khuey)
Attachment #781360 - Flags: review+
Attachment #781360 - Flags: feedback+
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?
Flags: in-testsuite-
[er, s/mozmemory/mozalloc/]
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.
https://hg.mozilla.org/mozilla-central/rev/924c4d59db8b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.