[vlc-devel] [PATCH] stream: error out on a failed conversion

[Thomas Guillem]

On Fri, Jun 23, 2017, at 17:59, Thomas Guillem wrote:
> 
> 
> On Fri, Jun 23, 2017, at 17:55, Rémi Denis-Courmont wrote:
> > On vendredi 23 juin 2017 15:27:53 EEST Shaleen wrote:
> > > From 96521880d5cd05a6ebba077dde47cc9c26c1d008 Mon Sep 17 00:00:00 2001
> > > From: Shaleen Jain <shaleen.jain95 at gmail.com>
> > > Date: Fri, 23 Jun 2017 15:26:40 +0530
> > > Subject: [PATCH] stream: error out on a failed conversion
> > > 
> > > ---
> > >  src/input/stream.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/input/stream.c b/src/input/stream.c
> > > index affc7b2c0d..a7ea3bfa43 100644
> > > --- a/src/input/stream.c
> > > +++ b/src/input/stream.c
> > > @@ -327,10 +327,12 @@ char *vlc_stream_ReadLine( stream_t *s )
> > >              p_in = p_line;
> > >              p_out = psz_new_line;
> > > 
> > > -            if( vlc_iconv( priv->text.conv, &p_in, &i_in, &p_out, &i_out )
> > > == (size_t)-1 )
> > > +            if( priv->text.conv == (vlc_iconv_t)-1 ||
> > 
> > AFAICT, that's logically impossible now.
> 
> Ah ? I think it's possible if you first call vlc_stream_ReadLine() after
> a seek.
> (OK, you have to be dumb to do that, so maybe we can put an assert
> instead)

Ah yes, it's now impossible since this code is executed only if
text.char_width > 1, and this is the case only if iconv is initialized.

> 
> > 
> > > +                    vlc_iconv( priv->text.conv, &p_in, &i_in, &p_out,
> > > &i_out ) == (size_t)-1 )
> > >              {
> > >                  msg_Err( s, "conversion error: %s", vlc_strerror_c( errno
> > > ) );
> > >                  msg_Dbg( s, "original: %d, in %zu, out %zu", i_line, i_in,
> > > i_out );
> > > +                goto error;
> > 
> > This looks OK. But there are less extreme ways to deal with an invalid 
> > sequence of UTF-16 surrogates than to drop the whole line.
> > 
> > >              }
> > >              free( p_line );
> > >              p_line = psz_new_line;
> > > 
> > > > On Fri, Jun 23, 2017, at 13:29, Shaleen Jain wrote:
> > > > > ---
> > > > > 
> > > > >  src/input/stream.c | 5 ++++-
> > > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/src/input/stream.c b/src/input/stream.c
> > > > > index affc7b2c0d..efc959319c 100644
> > > > > --- a/src/input/stream.c
> > > > > +++ b/src/input/stream.c
> > > > > @@ -195,6 +195,7 @@ char *vlc_stream_ReadLine( stream_t *s )
> > > > > 
> > > > >          if( i_pos == 0 && i_data >= 2 )
> > > > >          {
> > > > >          
> > > > >              const char *psz_encoding = NULL;
> > > > > 
> > > > > +            priv->text.conv = (vlc_iconv_t)-1;
> > > > 
> > > > priv->text.conv is already initialized from vlc_stream_CommonNew(), why
> > > > re-initializing it here ?
> > > > 
> > > > >              if( unlikely(priv->text.conv != (vlc_iconv_t)-1) )
> > > > >              {   /* seek back to beginning? reset */
> > > > > 
> > > > > @@ -327,10 +328,12 @@ char *vlc_stream_ReadLine( stream_t *s )
> > > > > 
> > > > >              p_in = p_line;
> > > > >              p_out = psz_new_line;
> > > > > 
> > > > > -            if( vlc_iconv( priv->text.conv, &p_in, &i_in, &p_out,
> > > > > &i_out
> > > > > ) == (size_t)-1 )
> > > > > +            if( priv->text.conv == (vlc_iconv_t)-1 ||
> > > > > +                    vlc_iconv( priv->text.conv, &p_in, &i_in, &p_out,
> > > > > &i_out ) == (size_t)-1 )
> > > > > 
> > > > >              {
> > > > >              
> > > > >                  msg_Err( s, "conversion error: %s", vlc_strerror_c(
> > > > >                  errno ) );
> > > > >                  msg_Dbg( s, "original: %d, in %zu, out %zu", i_line,
> > > > >                  i_in, i_out );
> > > > > 
> > > > > +                goto error;
> > > > 
> > > > OK for me.
> > > > 
> > > > >              }
> > > > >              free( p_line );
> > > > >              p_line = psz_new_line;
> > > > > 
> > > > > --
> > > > > 2.13.1
> > > > > 
> > > > > _______________________________________________
> > > > > vlc-devel mailing list
> > > > > To unsubscribe or modify your subscription options:
> > > > > https://mailman.videolan.org/listinfo/vlc-devel
> > > > 
> > > > _______________________________________________
> > > > vlc-devel mailing list
> > > > To unsubscribe or modify your subscription options:
> > > > https://mailman.videolan.org/listinfo/vlc-devel
> > 
> > 
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel