<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>