On Tue, Mar 13, 2012 at 02:56:07PM +0100, Hans de Goede wrote:
> Hi,
>
> On 03/13/2012 02:40 PM, Daniel P. Berrange wrote:
> >From: "Daniel P. Berrange"<berra...@redhat.com>
> >
> >A few functions have very large arrays declared on the stack.
> >Replace these with heap allocations, to reduce risk of stack
> >overflows in deep callpaths
> >---
> > gtk/channel-playback.c | 6 ++++--
> > gtk/spice-channel.c | 16 ++++++++++++----
> > 2 files changed, 16 insertions(+), 6 deletions(-)
> >
> >diff --git a/gtk/channel-playback.c b/gtk/channel-playback.c
> >index 32f8b1a..c353cd2 100644
> >--- a/gtk/channel-playback.c
> >+++ b/gtk/channel-playback.c
> >@@ -353,10 +353,12 @@ static void playback_handle_data(SpiceChannel
> >*channel, SpiceMsgIn *in)
> > packet->data, packet->data_size);
> > break;
> > case SPICE_AUDIO_DATA_MODE_CELT_0_5_1: {
> >- celt_int16_t pcm[256 * 2];
> >+ celt_int16_t *pcm;
> >+ gsize pcmLen = 256 * 2;
> >
> > g_return_if_fail(c->celt_decoder != NULL);
> >
> >+ pcm = g_new0(celt_int16_t, pcmLen);
> > if (celt051_decode(c->celt_decoder, packet->data,
> > packet->data_size, pcm) != CELT_OK) {
> > g_warning("celt_decode() error");
> >@@ -364,7 +366,7 @@ static void playback_handle_data(SpiceChannel *channel,
> >SpiceMsgIn *in)
> > }
> >
> > emit_main_context(channel, SPICE_PLAYBACK_DATA,
> >- (uint8_t *)pcm, sizeof(pcm));
> >+ (uint8_t *)pcm, pcmLen);
> > break;
> > }
> > default:
>
> You seem to be never freeing the data here. Also I disagree with switching
> to malloc / free here in general. This path will get called many times a
> second,
> malloc / free is not a cheap operation and 1024 bytes is not such a big
> amount to
> put on the stack.
[snip]
> This one looks fine from the free pov. But again this is a semi hot
> code path, and malloc / free is not a cheap operation, also there are
> no guarantees wrt cache hotness when it comes to the heap, where as the stack
> is likely hot in the cache.
Ok, no worries, I can tweak the limit in m4/spice-compile.warnings.m4 to
allow these larger stack allocations.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
