<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi Rémi,</p>
<p>Sorry for taking a while to reply to your message; I must have accidentally marked it as read during my most recent flight, and only now discovered that I never took any action regarding it.</p>
<hr style="height:1px;margin-bottom:20px;background-color:#ddd;color:#ddd" />
<p>On 2017-03-01 16:14, Rémi Denis-Courmont wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> On March 1, 2017 10:52:09 AM GMT+02:00, "Filip Roséen" <filip@atch.se> wrote:</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code>--- a/modules/access/satip.c
+++ b/modules/access/satip.c
@@ -645,10 +645,8 @@ static int satip_open(vlc_object_t *obj)
for (unsigned i = 0; i < strlen(psz_lower_url); i++)
psz_lower_url[i] = tolower(psz_lower_url[i]);
- const char* psz_lower_location = strstr( psz_lower_url, "://" );
- if ( psz_lower_location == NULL )
+ if( strstr( psz_lower_url, "://" ) == NULL )
goto error;
- psz_lower_location += 3;</code></pre>
</blockquote>
<pre><code> Valid point but isn't the strstr() too redundant with UrlParse?</code></pre>
</blockquote>
<p>Regarding the usage of <code>strstr</code> I was reluctant to remove it as I feared that it would be a change of behavior, but as <code>access->psz_url</code> is guaranteed to contain the searched for substring (because of the check in <code>src/input/access.c:access_New</code> which also looks for <code>://</code>) it is indeed redundant.</p>
<p>Given that the <em>return-value</em> of <code>vlc_UrlParse</code> is not checked, nor the actual state of the resulting <code>struct</code> is properly checked for errors, the redundancy should not be because of usage of the function.</p>
<p>Attach to this email is a patch that removes the useless code, but I will follow up with more fixes for the module in question as soon as possible (see next section).</p>
<p>Best Regards,<br />
Filip</p>
<hr style="height:1px;margin-bottom:20px;background-color:#ddd;color:#ddd" />
<h3 id="further-thoughts">Further thoughts</h3>
<p>As it turns out, <code>satip.c:Open</code> needs to be fixed far deeper than removing a write-only variable. Your messaged caused me to look at the implementation in more detail, and as it turns out there are numerous ways in which the code will invoke <em>undefined-behavior</em> due to erroneous assumptions.</p>
</body>
</html>