<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body><div><br></div>
<div>On Fri, Jun 23, 2017, at 12:10, Shaleen Jain wrote:<br></div>
<blockquote type="cite"><div>From d274065aeb20b6538a108bb310e7c8ab2445e5fd Mon Sep 17 00:00:00 2001<br></div>
<div>From: Shaleen Jain <shaleen.jain95@gmail.com><br></div>
<div>Date: Fri, 23 Jun 2017 15:26:40 +0530<br></div>
<div>Subject: [PATCH] stream: iconv: convert only if we have a conversion descriptor<br></div>
<div><br></div>
<div>---<br></div>
<div> src/input/stream.c | 6 ++++--<br></div>
<div> 1 file changed, 4 insertions(+), 2 deletions(-)<br></div>
<div><br></div>
<div>diff --git a/src/input/stream.c b/src/input/stream.c<br></div>
<div>index affc7b2c0d..ecdaa2f997 100644<br></div>
<div>--- a/src/input/stream.c<br></div>
<div>+++ b/src/input/stream.c<br></div>
<div>@@ -195,6 +195,7 @@ char *vlc_stream_ReadLine( stream_t *s )<br></div>
<div>         if( i_pos == 0 && i_data >= 2 )<br></div>
<div>         {<br></div>
<div>             const char *psz_encoding = NULL;<br></div>
<div>+          priv->text.conv = (vlc_iconv_t)-1;<br></div>
<div> <br></div>
<div>             if( unlikely(priv->text.conv != (vlc_iconv_t)-1) )<br></div>
<div>             {   /* seek back to beginning? reset */<br></div>
<div>@@ -220,7 +221,7 @@ char *vlc_stream_ReadLine( stream_t *s )<br></div>
<div>                 if( unlikely(priv->text.conv == (vlc_iconv_t)-1) )<br></div>
<div>                 {<br></div>
<div>                     msg_Err( s, "iconv_open failed" );<br></div>
<div>-                    goto error;<br></div>
<div>+            <span style="white-space:pre-wrap;"> </span>return NULL;<br></div>
<div>                 }<br></div>
<div>                 priv->text.char_width = 2;<br></div>
<div>             }<br></div>
<div>@@ -327,7 +328,8 @@ char *vlc_stream_ReadLine( stream_t *s )<br></div>
<div>             p_in = p_line;<br></div>
<div>             p_out = psz_new_line;<br></div>
<div> <br></div>
<div>-            if( vlc_iconv( priv->text.conv, &p_in, &i_in, &p_out, &i_out ) == (size_t)-1 )<br></div>
<div>+          if( priv->text.conv != (vlc_iconv_t)-1 &&<br></div>
<div>+            <span style="white-space:pre;"> </span>        vlc_iconv( priv->text.conv, &p_in, &i_in, &p_out, &i_out ) == (size_t)-1 )<br></div>
<div>             {<br></div>
<div>                 msg_Err( s, "conversion error: %s", vlc_strerror_c( errno ) );<br></div>
<div>                 msg_Dbg( s, "original: %d, in %zu, out %zu", i_line, i_in, i_out );<br></div>
</blockquote><div><br></div>
<div>OK but this is still not sane, you should add a goto error just after the errors logs. It's better to return nothing here than something that is badly or not converted.<br></div>
<div><br></div>
<blockquote type="cite"><div>-- <br></div>
<div>2.13.1<br></div>
<div><br></div>
<div>On Thu, Jun 22, 2017 at 5:45 PM, Rémi Denis-Courmont <remi@remlab.net> wrote:<br></div>
<div> <br></div>
<blockquote type="cite"><div defang_data-gmailquote="yes"><div>Le 22 juin 2017 14:44:49 GMT+03:00, Shaleen Jain <shaleen.jain95@gmail.com> a écrit :<br></div>
<blockquote defang_data-gmailquote="yes" style="margin-top:0pt;margin-right:0pt;margin-bottom:0pt;margin-left:0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204, 204, 204);padding-left:1ex;"><div><br></div>
<div>On Wed, Jun 21, 2017 at 11:38 PM, Rémi Denis-Courmont <remi@remlab.net> wrote:<br></div>
<div> <br></div>
<blockquote type="cite"><div style="white-space:pre-wrap;"><div>Le keskiviikkona 21. kesäkuuta 2017, 22.55.37 EEST Shaleen Jain a écrit : <br></div>
<blockquote>How so?<br></blockquote><div>It´s still not resulting in anything sane.<br></div>
</div>
</blockquote><div>I'm sorry I don't see how the logic is incorrect, <br></div>
<div>if priv->text.conv is -1 (which will happen if vlc_iconv_open() fails)<br></div>
<div><div>priv->text.conv != (vlc_iconv_t)(-1) would be false and vlc_iconv() would not run, otherwise it would proceed to do the conversion.<br></div>
<blockquote type="cite"><div style="white-space:pre-wrap;"><div>--<br></div>
<div>雷米‧德尼-库尔蒙 <a href="https://www.remlab.net/">https://www.remlab.net/</a> _______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options: <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div>
</div>
</blockquote><div><br></div>
<div><br></div>
</div>
</blockquote></div>
<div><br></div>
<div>Skipping the conversion is not going to result in sane behaviour.<br></div>
</blockquote><div>It isn't suppose to do anything sane by itself, it's a sanity check for when psz_encoding is null and the program doesn't need to do a conversion.<br></div>
<div><br></div>
<div>Also "goto error" when iconv_open fails is going to result in a invalid free of p_line.<br></div>
</blockquote><div><br></div>
<div>free(NULL) is valid, it's a NO-OP, we do it a lot in the VLC codebase.<br></div>
<div><br></div>
<blockquote type="cite"><div><br></div>
<div><div>Besides if you are not going to provide any constructive review and take someone else's patches and try to implement it yourself, without posting on the mailing list and be counter-productive then you might just as well do a fork.<br></div>
</div>
</blockquote><div><br></div>
<div>This is not an acceptable way of talking to one of the biggest contributors of the VLC project (and the same for every others members).<br></div>
<div><br></div>
<blockquote type="cite"><div><blockquote type="cite"><div><br></div>
<div>--<br></div>
<div> Rémi Denis-Courmont<br></div>
<div> Typed on an inconvenient virtual keyboard<br></div>
</blockquote><div><br></div>
</div>
<div><u>_______________________________________________</u><br></div>
<div>vlc-devel mailing list<br></div>
<div>To unsubscribe or modify your subscription options:<br></div>
<div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div>
</blockquote><div><br></div>
</body>
</html>