[0/1] Dead stores in maple-tree

  Dear maple-tree authors, dear Liam, dear Matthew,

there are some Dead Stores that clang-analyzer reports:

lib/maple_tree.c:2906:2: warning: Value stored to 'last' is never read [clang-analyzer-deadcode.DeadStores]
lib/maple_tree.c:2907:2: warning: Value stored to 'prev_min' is never read [clang-analyzer-deadcode.DeadStores]

I addressed these two cases, which were most obvious and clear to fix;
see patch of this one-element series.

Further, clang-analyzer reports more, which I did not address:

lib/maple_tree.c:332:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
lib/maple_tree.c:337:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]

Unclear to me if the tool is wrong or right in its analysis here for the two functions above.

lib/maple_tree.c:1212:23: warning: Value stored to 'nodep' during its initialization is never read [clang-analyzer-deadcode.DeadStores]

A lot of pointer magic. Unclear to me if the tool is wrong or right in its analysis here.

lib/maple_tree.c:5014:5: warning: Value stored to 'count' is never read [clang-analyzer-deadcode.DeadStores]

Unclear if the code is intended as it is now.

In mas_anode_descend(), the variable count is really just assigned and used once
effectively. The second assignment is never read. So, the variable count could
just be removed in mas_anode_descend().


Maybe these further warnings are helpful to clean up the code or find an issue
that was overlooked so far.


Best regards,

Lukas


Lukas Bulwahn (1):
  lib: maple_tree: remove unneeded initialization in mtree_range_walk()

 lib/maple_tree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

* Lukas Bulwahn <lukas.bulwahn@gmail.com> [221026 08:01]:
> Dear maple-tree authors, dear Liam, dear Matthew,
> 
> there are some Dead Stores that clang-analyzer reports:
> 
> lib/maple_tree.c:2906:2: warning: Value stored to 'last' is never read [clang-analyzer-deadcode.DeadStores]
> lib/maple_tree.c:2907:2: warning: Value stored to 'prev_min' is never read [clang-analyzer-deadcode.DeadStores]
> 
> I addressed these two cases, which were most obvious and clear to fix;
> see patch of this one-element series.
> 
> Further, clang-analyzer reports more, which I did not address:
> 
> lib/maple_tree.c:332:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> lib/maple_tree.c:337:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> 
> Unclear to me if the tool is wrong or right in its analysis here for the two functions above.

The tool is correct but these aren't going anywhere.  They are compiled
out and are needed for the future.

> 
> lib/maple_tree.c:1212:23: warning: Value stored to 'nodep' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
> 
> A lot of pointer magic. Unclear to me if the tool is wrong or right in its analysis here.

Agreed, this is unclear.. I don't like it and it needs to be removed.
I'll send something out shortly. This was refactoring by the looks of it.

> 
> lib/maple_tree.c:5014:5: warning: Value stored to 'count' is never read [clang-analyzer-deadcode.DeadStores]
> 
> Unclear if the code is intended as it is now.
> 
> In mas_anode_descend(), the variable count is really just assigned and used once
> effectively. The second assignment is never read. So, the variable count could
> just be removed in mas_anode_descend().

This was probably left over from refactoring as well.  I will fix this
as well, thanks.

> 
> 
> Maybe these further warnings are helpful to clean up the code or find an issue
> that was overlooked so far.

Much appreciated,
Liam
  

On Wed, Oct 26, 2022 at 02:23:19PM +0000, Liam Howlett wrote:
> * Lukas Bulwahn <lukas.bulwahn@gmail.com> [221026 08:01]:
> > Dear maple-tree authors, dear Liam, dear Matthew,
> > 
> > there are some Dead Stores that clang-analyzer reports:
> > 
> > lib/maple_tree.c:2906:2: warning: Value stored to 'last' is never read [clang-analyzer-deadcode.DeadStores]
> > lib/maple_tree.c:2907:2: warning: Value stored to 'prev_min' is never read [clang-analyzer-deadcode.DeadStores]
> > 
> > I addressed these two cases, which were most obvious and clear to fix;
> > see patch of this one-element series.
> > 
> > Further, clang-analyzer reports more, which I did not address:
> > 
> > lib/maple_tree.c:332:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> > lib/maple_tree.c:337:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> > 
> > Unclear to me if the tool is wrong or right in its analysis here for the two functions above.
> 
> The tool is correct but these aren't going anywhere.  They are compiled
> out and are needed for the future.
> 

lib/maple_tree.c
   330  static inline void mte_set_full(const struct maple_enode *node)
   331  {
   332          node = (void *)((unsigned long)node & ~MAPLE_ENODE_NULL);
   333  }
   334  
   335  static inline void mte_clear_full(const struct maple_enode *node)
   336  {
   337          node = (void *)((unsigned long)node | MAPLE_ENODE_NULL);
   338  }

That code is really puzzling...  How far into the future before it starts
making sense?

regards,
dan carpenter
  

* Dan Carpenter <dan.carpenter@oracle.com> [221027 03:43]:
> On Wed, Oct 26, 2022 at 02:23:19PM +0000, Liam Howlett wrote:
> > * Lukas Bulwahn <lukas.bulwahn@gmail.com> [221026 08:01]:
> > > Dear maple-tree authors, dear Liam, dear Matthew,
> > > 
> > > there are some Dead Stores that clang-analyzer reports:
> > > 
> > > lib/maple_tree.c:2906:2: warning: Value stored to 'last' is never read [clang-analyzer-deadcode.DeadStores]
> > > lib/maple_tree.c:2907:2: warning: Value stored to 'prev_min' is never read [clang-analyzer-deadcode.DeadStores]
> > > 
> > > I addressed these two cases, which were most obvious and clear to fix;
> > > see patch of this one-element series.
> > > 
> > > Further, clang-analyzer reports more, which I did not address:
> > > 
> > > lib/maple_tree.c:332:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> > > lib/maple_tree.c:337:2: warning: Value stored to 'node' is never read [clang-analyzer-deadcode.DeadStores]
> > > 
> > > Unclear to me if the tool is wrong or right in its analysis here for the two functions above.
> > 
> > The tool is correct but these aren't going anywhere.  They are compiled
> > out and are needed for the future.
> > 
> 
> lib/maple_tree.c

~line 302:
/* Bit 1 indicates the root is a node */
#define MAPLE_ROOT_NODE                 0x02
/* maple_type stored bit 3-6 */
#define MAPLE_ENODE_TYPE_SHIFT          0x03
/* Bit 2 means a NULL somewhere below */
#define MAPLE_ENODE_NULL                0x04


>    330  static inline void mte_set_full(const struct maple_enode *node)
>    331  {
>    332          node = (void *)((unsigned long)node & ~MAPLE_ENODE_NULL);
>    333  }
>    334  
>    335  static inline void mte_clear_full(const struct maple_enode *node)
>    336  {
>    337          node = (void *)((unsigned long)node | MAPLE_ENODE_NULL);
>    338  }

Looking at the code.... the analysis is correct and these need to be
fixed.  Thanks Dan & Lukas.

> 
> That code is really puzzling...  How far into the future before it starts
> making sense?

If you want to know details like this, you can look at the comments in
the header and c file - that's where the development information
resides.  Information about a node is encoded in the last bits of that
nodes pointer - since they are aligned we can use a mask to restore the
pointer.  Internally I refer to nodes with encoded information as
maple_enodes.  This part is to do with finding out if there is a free
index within the range the node holds.  Think about searching for the
next available index for a unique identifier.

Thanks,
Liam