<h2 id="introduction">Introduction</h2>
<pre><code>/**
 * stream_t definition
 */

struct stream_t
{
        */ ... */

    /** 
     * Read directory.
     *
     * Callback to fill an item node from a directory
     * (see doc/browsing.txt for details).
     *
     * NULL if the stream is not a directory.
     */
    int         (*pf_readdir)(stream_t *, input_item_node_t *); 

        /* ... */
};</code></pre>
<p>Given the new, and awesome (thank you Remi), documentation in <code>include/vlc_stream.h</code> it is clear that we have two mechanisms to check whether a <code>stream_t*</code> refers to a directory or not - as such, one of them is redundant.</p>
<p>I propose that we resolve the issue by either;</p>
<ul>
<li><p>remove <code>STREAM_IS_DIRECTORY</code> and rely entirely on the state of <code>stream_t.pf_readdir</code>, or;</p></li>
<li><p>change the documented behavior (remove the constraint of <code>pf_readdir</code>), so that <code>STREAM_IS_DIRECTORY</code> is the mechanism to use.</p></li>
</ul>
<h3 id="discussion">Discussion</h3>
<p>The contents later in this post will contain my immediate reactions relevant to this issue, but the purpose of this post is of course to discuss the following:</p>
<ul>
<li>What should we do about <code>STREAM_IS_DIRECTORY</code> and the constraints documented for <code>stream_t.pf_readdir</code>?</li>
</ul>
<hr style="height:1px;margin-bottom:20px;background-color:#ddd;color:#ddd" />
<h3 id="note-s-pf_readdir-null-is-currently-b0rked">Note: <code>s->pf_readdir == NULL</code> is currently b0rked</h3>
<p>The currently documented behavior does not correspond to what we actually do when creating an access.</p>
<p>Below is the relevant snippet from <code>src/input/access.c</code>:</p>
<pre><code>stream_t *stream_AccessNew(vlc_object_t *parent, input_thread_t *input,
                           bool preparsing, const char *url)
{
        /* ... */

    if (access->pf_readdir != NULL)
        s->pf_readdir = AStreamReadDir;
    else
        s->pf_readdir = AStreamNoReadDir;

        /* ... */
}</code></pre>
<hr style="height:1px;margin-bottom:20px;background-color:#ddd;color:#ddd" />
<h2 id="immediate-reactions">Immediate reactions</h2>
<p>Given that we have two alternatives, this section will include my thoughts on the different paths we can decide to walk.</p>
<p>I strongly feel that we should not have two separate mechanisms for checking the same thing.</p>
<h3 id="implications-of-removing-stream_is_directory">Implications of removing <code>STREAM_IS_DIRECTORY</code></h3>
<ul>
<li><p><strong>Unable to privatize callbacks</strong></p>
<p>It will be more complicated to (in the future) move <code>pf_readdir</code> (and the related callbacks) currently within <code>struct stream_t</code> to a private area not accessible by entities, outside the core/the module itself, using a stream.</p></li>
<li><p><strong>The codebase currently favors <code>STREAM_IS_DIRECTORY</code></strong></p>
<p>As previously stated in the <em>Note</em>, <code>stream_AccessNew</code> must comply with the mentioned documented behavior (it currently does not, and has not since (at least) d2e07d11 (2015-08-25).</p></li>
<li><p><strong>Conditional paths on <code>s->pf_readdir</code> are non-existant</strong></p>
<p>As far as I can see, there is not a single place where the state of <code>s->pf_readdir</code> (where <code>s</code> can be any <code>stream_t*</code>) is used to check whether a stream is considered a directory or not.</p></li>
</ul>
<h3 id="implications-of-removing-the-constraints-of-pf_readdir">Implications of removing the constraints of <code>pf_readdir</code></h3>
<ul>
<li><p><strong>More difficult to check stream-“type”</strong></p>
<p>It will require additional logic in order to check whether a stream is a directory or not. <code>if( s->pf_readdir )</code> is of course easier to write than the below:</p>
<pre><code>bool b_isdir;

if( stream_Control( s, STREAM_IS_DIRECTORY, &b_isdir ) )
    b_isdir = false;

if( b_isdir )
{
    /* ... */
}</code></pre></li>
</ul>