>
> Hi,
>
> I know this was merged already but just one comment bellow:
>
> On Wed, Nov 18, 2015 at 03:42:26PM -0600, Jonathon Jongsma wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > +static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> > +quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
> > +{
> > + EncoderData *usr_data = &(((QuicData *)usr)->data);
> > + va_list ap;
>
> I would prefer some #define here as well to get the EncoderData..
>
> > +static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> > +lz_usr_error(LzUsrContext *usr, const char *fmt, ...)
> > +{
> > + EncoderData *usr_data = &(((LzData *)usr)->data);
> > + va_list ap;
>
> As it is used a lot..
>
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +glz_usr_error(GlzEncoderUsrContext *usr, const char *fmt, ...)
> > +{
> > + EncoderData *usr_data = &(((GlzData *)usr)->data);
> > + va_list ap;
>
> +1
>
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +quic_usr_warn(QuicUsrContext *usr, const char *fmt, ...)
> > +{
> > + EncoderData *usr_data = &(((QuicData *)usr)->data);
> > + va_list ap;
>
> +1
>
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +lz_usr_warn(LzUsrContext *usr, const char *fmt, ...)
> > +{
> > + EncoderData *usr_data = &(((LzData *)usr)->data);
> > + va_list ap;
>
> +1
>
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +glz_usr_warn(GlzEncoderUsrContext *usr, const char *fmt, ...)
> > +{
> > + EncoderData *usr_data = &(((GlzData *)usr)->data);
> > + va_list ap;
>
> +1
>
> > +static int quic_usr_more_space(QuicUsrContext *usr, uint32_t **io_ptr, int
> > rows_completed)
> > +{
> > + EncoderData *usr_data = &(((QuicData *)usr)->data);
> > + return encoder_usr_more_space(usr_data, (uint8_t **)io_ptr) /
> > sizeof(uint32_t);
> > +}
> > +
> > +static int lz_usr_more_space(LzUsrContext *usr, uint8_t **io_ptr)
> > +{
> > + EncoderData *usr_data = &(((LzData *)usr)->data);
> > + return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +
> > +static int glz_usr_more_space(GlzEncoderUsrContext *usr, uint8_t **io_ptr)
> > +{
> > + EncoderData *usr_data = &(((GlzData *)usr)->data);
> > + return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +
> > +static int jpeg_usr_more_space(JpegEncoderUsrContext *usr, uint8_t
> > **io_ptr)
> > +{
> > + EncoderData *usr_data = &(((JpegData *)usr)->data);
> > + return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +
> > +#ifdef USE_LZ4
> > +static int lz4_usr_more_space(Lz4EncoderUsrContext *usr, uint8_t **io_ptr)
> > +{
> > + EncoderData *usr_data = &(((Lz4Data *)usr)->data);
> > + return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +#endif
> > +
> > +static int zlib_usr_more_space(ZlibEncoderUsrContext *usr, uint8_t
> > **io_ptr)
> > +{
> > + EncoderData *usr_data = &(((ZlibData *)usr)->data);
> > + return encoder_usr_more_space(usr_data, io_ptr);
> > +}
>
> +6
>
> > +static int quic_usr_more_lines(QuicUsrContext *usr, uint8_t **lines)
> > +{
> > + EncoderData *usr_data = &(((QuicData *)usr)->data);
> > + return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +static int lz_usr_more_lines(LzUsrContext *usr, uint8_t **lines)
> > +{
> > + EncoderData *usr_data = &(((LzData *)usr)->data);
> > + return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +static int glz_usr_more_lines(GlzEncoderUsrContext *usr, uint8_t **lines)
> > +{
> > + EncoderData *usr_data = &(((GlzData *)usr)->data);
> > + return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +static int jpeg_usr_more_lines(JpegEncoderUsrContext *usr, uint8_t
> > **lines)
> > +{
> > + EncoderData *usr_data = &(((JpegData *)usr)->data);
> > + return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +#ifdef USE_LZ4
> > +static int lz4_usr_more_lines(Lz4EncoderUsrContext *usr, uint8_t **lines)
> > +{
> > + EncoderData *usr_data = &(((Lz4Data *)usr)->data);
> > + return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +#endif
> > +
> > +static int zlib_usr_more_input(ZlibEncoderUsrContext *usr, uint8_t**
> > input)
> > +{
> > + EncoderData *usr_data = &(((ZlibData *)usr)->data);
> > + int buf_size;
>
> +6
>
> Cheers,
> - toso
I think that being a move was not good to add other stuff.
However I agree with a patch for these.
Personally I would like to see a macro like
#define GET_USR_DATA(type) &(SPICE_CONTAINEROF(usr, JpegData, usr)->data)
and something like
return encoder_usr_more_space(GET_USR_DATA(JpegData), io_ptr);
Looks like now is easier to integrate patches as the bigger moves
was integrated in master.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel