Author: cboesch Date: Sat Feb 26 12:58:53 2011 New Revision: 32973 Log: Use mp_asprintf in get_metadata instead of limited stack buffer. Modified: trunk/mplayer.c Modified: trunk/mplayer.c ============================================================================== --- trunk/mplayer.c Sat Feb 26 12:57:54 2011 (r32972) +++ trunk/mplayer.c Sat Feb 26 12:58:53 2011 (r32973) @@ -438,7 +438,6 @@ static char *get_demuxer_info (char *tag } char *get_metadata (metadata_t type) { - char meta[128]; sh_audio_t * const sh_audio = mpctx->sh_audio; sh_video_t * const sh_video = mpctx->sh_video; @@ -460,18 +459,14 @@ char *get_metadata (metadata_t type) { else if (sh_video->format == 0x10000005) return strdup("h264"); else if (sh_video->format >= 0x20202020) - snprintf(meta, sizeof(meta), "%.4s", (char *) &sh_video->format); - else - snprintf(meta, sizeof(meta), "0x%08X", sh_video->format); - return strdup(meta); + return mp_asprintf("%.4s", (char *)&sh_video->format); + return mp_asprintf("0x%08X", sh_video->format); case META_VIDEO_BITRATE: - snprintf(meta, sizeof(meta), "%d kbps", (int) (sh_video->i_bps * 8 / 1024)); - return strdup(meta); + return mp_asprintf("%d kbps", (int)(sh_video->i_bps * 8 / 1024)); case META_VIDEO_RESOLUTION: - snprintf(meta, sizeof(meta), "%d x %d", sh_video->disp_w, sh_video->disp_h); - return strdup(meta); + return mp_asprintf("%d x %d", sh_video->disp_w, sh_video->disp_h); case META_AUDIO_CODEC: if (sh_audio->codec && sh_audio->codec->name) @@ -479,12 +474,10 @@ char *get_metadata (metadata_t type) { break; case META_AUDIO_BITRATE: - snprintf(meta, sizeof(meta), "%d kbps", (int)(sh_audio->i_bps * 8 / 1000)); - return strdup(meta); + return mp_asprintf("%d kbps", (int)(sh_audio->i_bps * 8 / 1000)); case META_AUDIO_SAMPLES: - snprintf(meta, sizeof(meta), "%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels); - return strdup(meta); + return mp_asprintf("%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels); /* check for valid demuxer */ case META_INFO_TITLE:
On Sat, Feb 26, 2011 at 12:58:53 (CET), cboesch wrote:
Author: cboesch Date: Sat Feb 26 12:58:53 2011 New Revision: 32973
Log: Use mp_asprintf in get_metadata instead of limited stack buffer.
Modified: trunk/mplayer.c
Modified: trunk/mplayer.c ============================================================================== --- trunk/mplayer.c Sat Feb 26 12:57:54 2011 (r32972) +++ trunk/mplayer.c Sat Feb 26 12:58:53 2011 (r32973) @@ -438,7 +438,6 @@ static char *get_demuxer_info (char *tag }
char *get_metadata (metadata_t type) { - char meta[128]; sh_audio_t * const sh_audio = mpctx->sh_audio; sh_video_t * const sh_video = mpctx->sh_video;
@@ -460,18 +459,14 @@ char *get_metadata (metadata_t type) { else if (sh_video->format == 0x10000005) return strdup("h264"); else if (sh_video->format >= 0x20202020) - snprintf(meta, sizeof(meta), "%.4s", (char *) &sh_video->format); - else - snprintf(meta, sizeof(meta), "0x%08X", sh_video->format); - return strdup(meta); + return mp_asprintf("%.4s", (char *)&sh_video->format); + return mp_asprintf("0x%08X", sh_video->format);
case META_VIDEO_BITRATE: - snprintf(meta, sizeof(meta), "%d kbps", (int) (sh_video->i_bps * 8 / 1024)); - return strdup(meta); + return mp_asprintf("%d kbps", (int)(sh_video->i_bps * 8 / 1024));
case META_VIDEO_RESOLUTION: - snprintf(meta, sizeof(meta), "%d x %d", sh_video->disp_w, sh_video->disp_h); - return strdup(meta); + return mp_asprintf("%d x %d", sh_video->disp_w, sh_video->disp_h);
case META_AUDIO_CODEC: if (sh_audio->codec && sh_audio->codec->name) @@ -479,12 +474,10 @@ char *get_metadata (metadata_t type) { break;
case META_AUDIO_BITRATE: - snprintf(meta, sizeof(meta), "%d kbps", (int)(sh_audio->i_bps * 8 / 1000)); - return strdup(meta); + return mp_asprintf("%d kbps", (int)(sh_audio->i_bps * 8 / 1000));
case META_AUDIO_SAMPLES: - snprintf(meta, sizeof(meta), "%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels); - return strdup(meta); + return mp_asprintf("%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels);
/* check for valid demuxer */ case META_INFO_TITLE:
This commit breaks the daily mplayer builds on amd64: Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Our automated build log filter detected the problem(s) above that will likely cause your package to segfault on architectures where the size of a pointer is greater than the size of an integer, such as ia64 and amd64. This is often due to a missing function prototype definition. Since use of implicitly converted pointers is always fatal to the application on ia64, they are errors. Please correct them for your next upload. More information can be found at: http://wiki.debian.org/ImplicitPointerConversions Full buildlog here: http://launchpadlibrarian.net/65218896/buildlog_ubuntu-maverick-amd64.mplaye... -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4
On Sun, Feb 27, 2011 at 08:10:06AM +0100, Reinhard Tartler wrote:
On Sat, Feb 26, 2011 at 12:58:53 (CET), cboesch wrote:
Author: cboesch Date: Sat Feb 26 12:58:53 2011 New Revision: 32973
Log: Use mp_asprintf in get_metadata instead of limited stack buffer.
Modified: trunk/mplayer.c
Modified: trunk/mplayer.c ============================================================================== --- trunk/mplayer.c Sat Feb 26 12:57:54 2011 (r32972) +++ trunk/mplayer.c Sat Feb 26 12:58:53 2011 (r32973) @@ -438,7 +438,6 @@ static char *get_demuxer_info (char *tag }
char *get_metadata (metadata_t type) { - char meta[128]; sh_audio_t * const sh_audio = mpctx->sh_audio; sh_video_t * const sh_video = mpctx->sh_video;
@@ -460,18 +459,14 @@ char *get_metadata (metadata_t type) { else if (sh_video->format == 0x10000005) return strdup("h264"); else if (sh_video->format >= 0x20202020) - snprintf(meta, sizeof(meta), "%.4s", (char *) &sh_video->format); - else - snprintf(meta, sizeof(meta), "0x%08X", sh_video->format); - return strdup(meta); + return mp_asprintf("%.4s", (char *)&sh_video->format); + return mp_asprintf("0x%08X", sh_video->format);
case META_VIDEO_BITRATE: - snprintf(meta, sizeof(meta), "%d kbps", (int) (sh_video->i_bps * 8 / 1024)); - return strdup(meta); + return mp_asprintf("%d kbps", (int)(sh_video->i_bps * 8 / 1024));
case META_VIDEO_RESOLUTION: - snprintf(meta, sizeof(meta), "%d x %d", sh_video->disp_w, sh_video->disp_h); - return strdup(meta); + return mp_asprintf("%d x %d", sh_video->disp_w, sh_video->disp_h);
case META_AUDIO_CODEC: if (sh_audio->codec && sh_audio->codec->name) @@ -479,12 +474,10 @@ char *get_metadata (metadata_t type) { break;
case META_AUDIO_BITRATE: - snprintf(meta, sizeof(meta), "%d kbps", (int)(sh_audio->i_bps * 8 / 1000)); - return strdup(meta); + return mp_asprintf("%d kbps", (int)(sh_audio->i_bps * 8 / 1000));
case META_AUDIO_SAMPLES: - snprintf(meta, sizeof(meta), "%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels); - return strdup(meta); + return mp_asprintf("%d Hz, %d ch.", sh_audio->samplerate, sh_audio->channels);
/* check for valid demuxer */ case META_INFO_TITLE:
This commit breaks the daily mplayer builds on amd64:
Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462
He fixed it. Missing function prototypes probably should be made an error, as for FFmpeg.
On Sun, Feb 27, 2011 at 02:17:28PM +0100, Reimar Döffinger wrote:
This commit breaks the daily mplayer builds on amd64:
Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462 Function `mp_asprintf' implicitly converted to pointer at mplayer.c:462
He fixed it. Missing function prototypes probably should be made an error, as for FFmpeg.
Yes sorry about that. Another solution would be to fix all the warnings so we can notice them when there is new ones… -- Clément B.
Clément Boesch wrote on Sun, 27 Feb 2011 14:28:34 +0100:
Another solution would be to fix all the warnings
Agreed, and with compiler instruction outputs suppressed they will stand out like a sore thumb. :) Ingo
On 27 Feb 2011, at 17:08, Ingo Brückl <ib@wupperonline.de> wrote:
Clément Boesch wrote on Sun, 27 Feb 2011 14:28:34 +0100:
Another solution would be to fix all the warnings
Agreed, and with compiler instruction outputs suppressed they will stand out like a sore thumb. :)
And if we're really lucky that might actually happen in the next few months. Much more likely though some warnings will still be there in years and strategically making some errors will at least avoid them becoming more and it will work right now.
On Sun, Feb 27, 2011 at 05:08:51PM +0100, Ingo Brückl wrote:
Clément Boesch wrote on Sun, 27 Feb 2011 14:28:34 +0100:
Another solution would be to fix all the warnings
Agreed, and with compiler instruction outputs suppressed they will stand out like a sore thumb. :)
Patches welcome. Diego
Diego Biurrun wrote on Sun, 06 Mar 2011 16:06:10 +0100:
On Sun, Feb 27, 2011 at 05:08:51PM +0100, Ingo Brückl wrote:
Clément Boesch wrote on Sun, 27 Feb 2011 14:28:34 +0100:
Another solution would be to fix all the warnings
Agreed, and with compiler instruction outputs suppressed they will stand out like a sore thumb. :)
Patches welcome.
Here you go. Ingo
I wrote on Sun, 06 Mar 2011 21:15:44 +0100:
mencoder$(EXESUF) mplayer$(EXESUF): - $(CC) -o $@ $^ $(EXTRALIBS) + @echo "LD " $< + @$(CC) -o $@ $^ $(EXTRALIBS)
Argh! I shouldn't make changes after testing (in this case to follow the ffmpeg output). It's @echo "LD " $@ of course. Ingo
On Mon, Mar 07, 2011 at 12:08:01PM +0100, Ingo Brückl wrote:
I wrote on Sun, 06 Mar 2011 21:15:44 +0100:
mencoder$(EXESUF) mplayer$(EXESUF): - $(CC) -o $@ $^ $(EXTRALIBS) + @echo "LD " $< + @$(CC) -o $@ $^ $(EXTRALIBS)
Argh! I shouldn't make changes after testing (in this case to follow the ffmpeg output). It's
@echo "LD " $@
of course.
btw, why not a tab instead of those spaces? There is exactly 8 char with LD + spaces, so a tab should give you the same output. -- Clément B.
Clément Boesch wrote on Mon, 7 Mar 2011 12:16:23 +0100:
On Mon, Mar 07, 2011 at 12:08:01PM +0100, Ingo Brückl wrote:
I wrote on Sun, 06 Mar 2011 21:15:44 +0100:
@echo "LD " $@
btw, why not a tab instead of those spaces? There is exactly 8 char with LD + spaces, so a tab should give you the same output.
You're right, I didn't notice. Actually, ffmpeg uses printf "LD\t%s\n" $@ Ingo
Diego Biurrun wrote on Sun, 06 Mar 2011 16:06:10 +0100:
On Sun, Feb 27, 2011 at 05:08:51PM +0100, Ingo Brückl wrote:
Clément Boesch wrote on Sun, 27 Feb 2011 14:28:34 +0100:
Another solution would be to fix all the warnings
Agreed, and with compiler instruction outputs suppressed they will stand out like a sore thumb. :)
Patches welcome.
Improved patch. Should suppress all outputs now. Ingo
On Wed, Mar 09, 2011 at 04:52:44PM +0100, Ingo Brückl wrote:
Diego Biurrun wrote on Sun, 06 Mar 2011 16:06:10 +0100:
On Sun, Feb 27, 2011 at 05:08:51PM +0100, Ingo Brückl wrote:
Clément Boesch wrote on Sun, 27 Feb 2011 14:28:34 +0100:
Another solution would be to fix all the warnings
Agreed, and with compiler instruction outputs suppressed they will stand out like a sore thumb. :)
Patches welcome.
Improved patch. Should suppress all outputs now.
This must be conditionnal and documented: make V=1 for instance will display the full compilation (which can be quite useful). Check how it's done in FFmpeg. Note I like this feature, but it must be done correctly. -- Clément B.
On Wed, Mar 09, 2011 at 04:52:44PM +0100, Ingo Brückl wrote:
Diego Biurrun wrote on Sun, 06 Mar 2011 16:06:10 +0100:
On Sun, Feb 27, 2011 at 05:08:51PM +0100, Ingo Brückl wrote:
Clément Boesch wrote on Sun, 27 Feb 2011 14:28:34 +0100:
Another solution would be to fix all the warnings
Agreed, and with compiler instruction outputs suppressed they will stand out like a sore thumb. :)
Patches welcome.
Improved patch. Should suppress all outputs now.
You should really reuse the code from ffmpeg/common.mak, the one under "ifndef V"
Clément Boesch wrote on Wed, 9 Mar 2011 22:14:50 +0100:
This must be conditionnal and documented: make V=1 for instance will display the full compilation (which can be quite useful).
Here you go. Reimar Döffinger wrote on Wed, 9 Mar 2011 22:20:01 +0100:
You should really reuse the code from ffmpeg/common.mak, the one under "ifndef V"
I can't use the same code (due to codecs.conf.h and help_mp.h), but can do it similarly. Hope that meets the expectations. Ingo
On Thu, Mar 10, 2011 at 11:08:16AM +0100, Ingo Brückl wrote:
Clément Boesch wrote on Wed, 9 Mar 2011 22:14:50 +0100:
This must be conditionnal and documented: make V=1 for instance will display the full compilation (which can be quite useful).
Here you go.
Reimar Döffinger wrote on Wed, 9 Mar 2011 22:20:01 +0100:
You should really reuse the code from ffmpeg/common.mak, the one under "ifndef V"
I can't use the same code (due to codecs.conf.h and help_mp.h), but can do it similarly.
You'll have to explain that. I used almost the same code (at most minor changes) and it worked just fine. http://lists-archives.org/mplayer-dev-eng/30851-add-ffmpeg-style-quiet-compi...
Reimar Döffinger wrote on Thu, 10 Mar 2011 20:47:42 +0100:
On Thu, Mar 10, 2011 at 11:08:16AM +0100, Ingo Brückl wrote:
Reimar Döffinger wrote on Wed, 9 Mar 2011 22:20:01 +0100:
You should really reuse the code from ffmpeg/common.mak, the one under "ifndef V"
I can't use the same code (due to codecs.conf.h and help_mp.h), but can do it similarly.
You'll have to explain that. I used almost the same code (at most minor changes) and it worked just fine.
It's because codecs.conf.h and help_mp.h have different (more informational) printfs: printf "GEN\t%s, " $@ printf "GEN\t%s from %s %s\n" $@ $(HELP_FILE) $(CHARSET) If I'm wasting my time here, because this solution isn't desirable and/or acceptable, please just tell me. Ingo
On 11 Mar 2011, at 00:20, Ingo Brückl <ib@wupperonline.de> wrote:
Reimar Döffinger wrote on Thu, 10 Mar 2011 20:47:42 +0100:
On Thu, Mar 10, 2011 at 11:08:16AM +0100, Ingo Brückl wrote:
Reimar Döffinger wrote on Wed, 9 Mar 2011 22:20:01 +0100:
You should really reuse the code from ffmpeg/common.mak, the one under "ifndef V"
I can't use the same code (due to codecs.conf.h and help_mp.h), but can do it similarly.
You'll have to explain that. I used almost the same code (at most minor changes) and it worked just fine.
It's because codecs.conf.h and help_mp.h have different (more informational) printfs:
printf "GEN\t%s, " $@
printf "GEN\t%s from %s %s\n" $@ $(HELP_FILE) $(CHARSET)
If I'm wasting my time here, because this solution isn't desirable and/or acceptable, please just tell me.
IMO: no point in having special messages for them, If someone cares they can use V. But even if you want them, you can still use the FFmpeg code for .o files etc.
On Sun, Feb 27, 2011 at 02:28:34PM +0100, Clément Bœsch wrote:
Another solution would be to fix all the warnings so we can notice them when there is new ones…
Patches welcome. Diego
On Sun, Feb 27, 2011 at 02:17:28PM +0100, Reimar Döffinger wrote:
Missing function prototypes probably should be made an error, as for FFmpeg.
Patches welcome. Diego
On Sun, Mar 06, 2011 at 04:05:36PM +0100, Diego Biurrun wrote:
On Sun, Feb 27, 2011 at 02:17:28PM +0100, Reimar Döffinger wrote:
Missing function prototypes probably should be made an error, as for FFmpeg.
Patches welcome.
(using -Werror-implicit-function-declaration instead of -Werror=implicit-function-declaration should mean we do not have to check whether it is supported) Index: configure =================================================================== --- configure (revision 33029) +++ configure (working copy) @@ -2579,7 +2579,7 @@ CFLAGS="-O2 $_march $_mcpu $_pipe" else CFLAGS="-O4 $_march $_mcpu $_pipe -ffast-math -fomit-frame-pointer" - WARNFLAGS="-Wall -Wno-switch -Wno-parentheses -Wpointer-arith -Wredundant-decls" + WARNFLAGS="-Wall -Wno-switch -Wno-parentheses -Wpointer-arith -Wredundant-decls -Werror-implicit-function-declaration" extra_ldflags="$extra_ldflags -ffast-math" fi else
On Sun, Mar 06, 2011 at 04:37:55PM +0100, Reimar Döffinger wrote:
On Sun, Mar 06, 2011 at 04:05:36PM +0100, Diego Biurrun wrote:
On Sun, Feb 27, 2011 at 02:17:28PM +0100, Reimar Döffinger wrote:
Missing function prototypes probably should be made an error, as for FFmpeg.
Patches welcome.
(using -Werror-implicit-function-declaration instead of -Werror=implicit-function-declaration should mean we do not have to check whether it is supported)
You mean this will work with all gcc versions and not just the newer ones? Then we should port this to FFmpeg as well. Diego
On Sun, Mar 06, 2011 at 06:51:30PM +0100, Diego Biurrun wrote:
On Sun, Mar 06, 2011 at 04:37:55PM +0100, Reimar Döffinger wrote:
On Sun, Mar 06, 2011 at 04:05:36PM +0100, Diego Biurrun wrote:
On Sun, Feb 27, 2011 at 02:17:28PM +0100, Reimar Döffinger wrote:
Missing function prototypes probably should be made an error, as for FFmpeg.
Patches welcome.
(using -Werror-implicit-function-declaration instead of -Werror=implicit-function-declaration should mean we do not have to check whether it is supported)
You mean this will work with all gcc versions and not just the newer ones?
Yes, to my knowledge it should even work on 2.95, but I have not tested.
Then we should port this to FFmpeg as well.
Måns opposed it back then to my knowledge.
Reimar Döffinger <Reimar.Doeffinger <at> gmx.de> writes:
(using -Werror-implicit-function-declaration instead of -Werror=implicit-function-declaration should mean we do not have to check whether it is supported)
You mean this will work with all gcc versions and not just the newer ones?
Yes, to my knowledge it should even work on 2.95, but I have not tested.
Works fine with gcc 2.95.3 Carl Eugen
participants (7)
-
Carl Eugen Hoyos -
cboesch -
Clément Bœsch -
Diego Biurrun -
Ingo Brückl -
Reimar Döffinger -
Reinhard Tartler