Closed Bug 1237471 Opened 6 years ago Closed 6 years ago

New ffvpx code redefines vsnprintf and snprintf on windows unconditionally conflicting with VS2015

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: bas.schouten, Assigned: jya)

References

Details

Attachments

(2 files)

ffvpxcommon.mozbuild defines snprintf and vsnprintf. VS2015 actually has these functions as part of the standard library and it does not like us redefining them.
thanks for reporting that. will take over.
Assignee: bas → jyavenard
Component: Audio/Video → Build Config
https://reviewboard.mozilla.org/r/29799/#review26641

::: media/ffvpx/libavutil/avutil-54-oldmsvc.symbols:1
(Diff revision 1)
> +av_add_q

You could #include avutil-54.symbols instead of putting everything in here.
Comment on attachment 8704944 [details] [diff] [review]
Fix VS2015 compilation. rpending=glandium

Mike, I see that you're around.. If you could have a look quickly.

Verified fixed locally.

I'll push it shortly if you're not there...
Flags: needinfo?(mh+mozilla)
Comment on attachment 8704926 [details]
MozReview Request: Bug 1237471: Fix VS2015 build by not using custom snprintf/vsnprintf functions when having VS2015's. r=jya

I'd prefer not to have a second symbols file, and that will break all non-2015 build.

should have been:
if CONFIG['_MSC_VER'] && CONFIG['_MSC_VER'] < '1900'
Attachment #8704926 - Flags: review?(jyavenard)
https://hg.mozilla.org/mozilla-central/rev/e597b8795fbe
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment on attachment 8704944 [details] [diff] [review]
Fix VS2015 compilation. rpending=glandium

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

::: media/ffvpx/libavutil/dummy_funcs.c
@@ +20,5 @@
> +
> +// VS2015 (and later) now provides snprintf. As we have a global symbols
> +// file that do not allow conditional export, we create a dummy entry instead.
> +#if defined(_MSC_VER) && _MSC_VER >= 1900
> +int avpriv_snprintf(char *s, size_t n, const char *fmt, ...) { return 0; }

I hope this is not used...
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #9)
> Comment on attachment 8704944 [details] [diff] [review]
> Fix VS2015 compilation. rpending=glandium
> 
> Review of attachment 8704944 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: media/ffvpx/libavutil/dummy_funcs.c
> @@ +20,5 @@
> > +
> > +// VS2015 (and later) now provides snprintf. As we have a global symbols
> > +// file that do not allow conditional export, we create a dummy entry instead.
> > +#if defined(_MSC_VER) && _MSC_VER >= 1900
> > +int avpriv_snprintf(char *s, size_t n, const char *fmt, ...) { return 0; }
> 
> I hope this is not used...

It isn't... But as the .symbols file list it, we have to make a dummy one (better alternative would be to have conditional support in .symbols file, so the symbol could be within #if _MSC_VER < 1900 .. #endif)
Depends on: 1277155
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.