<p>On 16/07/20 11:25, Francois Cartegnie wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Actions that are not bound to a single entity are now displayed
 correctly (vlc 2.2.4 has been used as reference), with two minor
 differences:
     * ACTION_PAUSE will now be shown if the current item is being
       played, and;
     * ACTION_PLAY will not show if the item is already playing.
 -    if( !index.isValid() )
 -        return false;
 -
 -    const PLItem *item = getItem( index );
 +    vlc_playlist_locker pl_lock ( THEPL );
 +        
 +    bool const b_readonly = THEPL->p_root->i_flags & PLAYLIST_RO_FLAG;
 +    AbstractPLItem * item = VLCModel::getItem( index );
 And then you introduce NULL deref in most cases.</code></pre>
</blockquote>
<p>Could you please point to a case where “NULL deref” happens might/will happen? I might be missing something very trivial, but I honestly do not see it.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> +        case ACTION_EXPLORE:
 +        {
 +            return getURI( index ).startsWith( "file:///" );
 +        }
 You changed scheme without reason.</code></pre>
</blockquote>
<p>What scheme has been changed? the logic for ACTION_EXPLORE is the same as before. If you are talking about the introduction of “{ }”, I did such because of habits when writing switch-cases (to have symmetry between the different cases)</p>
<p>I was not aware of a policy saying that such should not be used for single-line handlers; and if you feel strongly that it should not be I have no problem with removing such.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> +        case ACTION_RENAMENODE:
 +        {
 +            if( index == rootIndex() || item == NULL )
 +                return false;
 +
 +            input_item_t* p_iitem = item->inputItem();
 +
 +            if( p_iitem == NULL )
 +                return false;
 +
 +            return p_iitem->i_type == ITEM_TYPE_NODE
 +                || p_iitem->i_type == ITEM_TYPE_DIRECTORY;
 +        }
 You removed readonly cases.
 You're not allowed to rename readonly nodes.</code></pre>
</blockquote>
<p>Oh, you are right; I will fix this as soon as possible!</p>
<p>Though I do not see any other case where the usage of “b_readonly” vs “item->readOnly()” has been left out.</p>