Re: [PATCH 3/3] Don't abort if encoder cannot be created

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2015-11-18 at 09:57 -0500, Frediano Ziglio wrote:
> > 
> > Hi,
> > 
> > On Tue, 2015-11-17 at 16:38 -0600, Jonathon Jongsma wrote:
> > > Instead of using spice_critical() when an encoder cannot be created, use
> > > a warning so that the server doesn't abort.
> > 
> > I haven't seen an abort because of it, and I think abort is correct,
> > otherwise
> > you may end up calling encode functions like zlib_encode(dcc->zlib,...
> > with dcc->zlib = NULL, so it will crash
> > 
> > Pavel
> > 
> 
> I agree, NACK!

Yeah, this is why I split out this patch. I think it's probably safer to abort
here as well. Otherwise we'll have to go through the rest of the code and ensure
we're checking for NULL every time we use one of these variables. Let's just
drop this patch for now.



> TODO list: make these structures lazy initialized and free when not needed.
> This will increase scalability.
> 
> Frediano
> 
> 
> > > ---
> > >  server/dcc-encoders.c | 24 +++++-------------------
> > >  1 file changed, 5 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> > > index 35edd41..42c3364 100644
> > > --- a/server/dcc-encoders.c
> > > +++ b/server/dcc-encoders.c
> > > @@ -292,10 +292,7 @@ static void dcc_init_quic(DisplayChannelClient *dcc)
> > >      dcc->quic_data.usr.more_lines = quic_usr_more_lines;
> > >  
> > >      dcc->quic = quic_create(&dcc->quic_data.usr);
> > > -
> > > -    if (!dcc->quic) {
> > > -        spice_critical("create quic failed");
> > > -    }
> > > +    spice_warn_if_fail(dcc->quic);
> > >  }
> > >  
> > >  static void dcc_init_lz(DisplayChannelClient *dcc)
> > > @@ -309,10 +306,7 @@ static void dcc_init_lz(DisplayChannelClient *dcc)
> > >      dcc->lz_data.usr.more_lines = lz_usr_more_lines;
> > >  
> > >      dcc->lz = lz_create(&dcc->lz_data.usr);
> > > -
> > > -    if (!dcc->lz) {
> > > -        spice_critical("create lz failed");
> > > -    }
> > > +    spice_warn_if_fail(dcc->lz);
> > >  }
> > >  
> > >  static void glz_usr_free_image(GlzEncoderUsrContext *usr,
> > >  GlzUsrImageContext
> > > *image)
> > > @@ -356,10 +350,7 @@ static void dcc_init_jpeg(DisplayChannelClient *dcc)
> > >      dcc->jpeg_data.usr.more_lines = jpeg_usr_more_lines;
> > >  
> > >      dcc->jpeg = jpeg_encoder_create(&dcc->jpeg_data.usr);
> > > -
> > > -    if (!dcc->jpeg) {
> > > -        spice_critical("create jpeg encoder failed");
> > > -    }
> > > +    spice_warn_if_fail(dcc->jpeg);
> > >  }
> > >  
> > >  #ifdef USE_LZ4
> > > @@ -370,9 +361,7 @@ static inline void dcc_init_lz4(DisplayChannelClient
> > > *dcc)
> > >  
> > >      dcc->lz4 = lz4_encoder_create(&dcc->lz4_data.usr);
> > >  
> > > -    if (!dcc->lz4) {
> > > -        spice_critical("create lz4 encoder failed");
> > > -    }
> > > +    spice_warn_if_fail(dcc->lz4);
> > >  }
> > >  #endif
> > >  
> > > @@ -382,10 +371,7 @@ static void dcc_init_zlib(DisplayChannelClient *dcc)
> > >      dcc->zlib_data.usr.more_input = zlib_usr_more_input;
> > >  
> > >      dcc->zlib = zlib_encoder_create(&dcc->zlib_data.usr,
> > > ZLIB_DEFAULT_COMPRESSION_LEVEL);
> > > -
> > > -    if (!dcc->zlib) {
> > > -        spice_critical("create zlib encoder failed");
> > > -    }
> > > +    spice_warn_if_fail(dcc->zlib);
> > >  }
> > >  
> > >  void dcc_encoders_init(DisplayChannelClient *dcc)
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]