967006 - clang build failure - stun_build.h:57:9: error: empty struct has size 0 in C, size 1 in C++

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Alex Xu]

34:57.52 In file included from /home/alex/gecko-dev/media/mtransport/nricectx.cpp:75:
34:57.52 In file included from /home/alex/gecko-dev/media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.h:41:
34:57.52 In file included from /home/alex/gecko-dev/media/mtransport/third_party/nICEr/src/stun/stun.h:64:
34:57.52 /home/alex/gecko-dev/media/mtransport/third_party/nICEr/src/stun/stun_build.h:57:9: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
34:57.52 typedef struct nr_stun_client_stun_keepalive_params_ {
34:57.52         ^

clang version 3.4 (tags/RELEASE_34/final)
Target: x86_64-pc-linux-gnu
Thread model: posix

Comment 1

[Eric Rescorla (:ekr)]

This seems like it could be fixed by adding UCHAR dummy;

Comment 2

[Alex Xu]

There is already such code:

typedef struct nr_stun_client_stun_keepalive_params_ {
#ifdef WIN32  // silly VC++ gives error if no members
    int dummy;
#endif
} nr_stun_client_stun_keepalive_params;

I "fix" by replacing with this code:

typedef struct nr_stun_client_stun_keepalive_params_ {
#if defined(WIN32) || defined(__clang__)
    // VC++ and clang give error and warning respectively if no members
    int dummy;
#endif
} nr_stun_client_stun_keepalive_params;

But I'm pretty sure there's a better way to do this.

Comment 3

[Daniel Holbert [:dholbert]]

(Note: As a temporary workaround, you should be able to get past this by putting
 ac_add_options --disable-webrtc
in your .mozconfig. (if you don't need WebRTC in your local build.))

Comment 4

[Alex Xu]

Attachment: patch v1

working patch attached.

I dunno if we can just delete this struct though, or use #pragma.

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 8372880 [details] [diff] [review]
patch v1

Thanks - this looks reasonable as a local workaround at least (particularly given that we already suppress a similar error in MSVC).

However, this is in third-party code, and we usually avoid modifying our in-tree copies of third-party code so that our modifications don't just get stomped when we upgrade to the next release of the third-party library.  (instead, we try to fix things upstream and then pull in the fixes, or something along those lines)

Not sure if that's the case for this file or not. It looks like the last thing to touch this file was bug 886120, which did make some substantial modifications to this code in our tree, so we might not be treating this file as delicately as we treat other 3rd-party code.

Hence, I'll optimistically tag :abr (who reviewed bug 886120) for review.

Comment 6

[Adam Roach [:abr]]

Comment on attachment 8372880 [details] [diff] [review]
patch v1

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

Sorry for the delay in getting to this. The solution looks just fine.

In terms of upstreaming this change, EKR and I have broad latitude to upstream stuff into nICEr, since he's the main code author and we both have commit rights to the repo. At the moment, the majority of (and probably only) development being done on this library is in the Mozilla tree. When we get a breather, the plan is to bundle all our improvements together and push them back upstream.

Comment 7

[Daniel Holbert [:dholbert]]

Alex, could you re-post with author info / commit message included in the headers, as described here:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Then, you can mark this as "checkin-needed" and someone can import & push your updated patch.

Comment 8

[Alex Xu]

Attachment: patch v1 hg format

Comment 9

[Alex Xu]

Attachment: patch v1 hg format take 2

apparently git-patch-to-hg-format outputs peculiar commit messages.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/195910dc7604

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/195910dc7604