[Spice-devel] [PATCH 3/3] Don't abort if encoder cannot be created

[Jonathon Jongsma]

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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >