Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Sun, 5 Nov 2023 23:57:43 +0000
From:   Al Viro <viro@zeniv.linux.org.uk>
To:     Joey Pabalinas <joeypabalinas@gmail.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] fs: dlm: Remove impossible to hit if statement
Message-ID: <20231105235743.GS1957730@ZenIV>
References: <CAFhcgOWDZ8+KiwmFRYnaBrqSHyeGDX20PgjGHXWK5UikZHPCmA@mail.gmail.com>
 <20231105231125.GR1957730@ZenIV>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231105231125.GR1957730@ZenIV>
Sender: Al Viro <viro@ftp.linux.org.uk>
Precedence: bulk

On Sun, Nov 05, 2023 at 11:11:25PM +0000, Al Viro wrote:
> Sure, if you hit even a single iteration of that loop, you will
> have memb guaranteed to be non-NULL.  Therefore, to complete the
> proof you only need to consider what happens if there is not
> a single iteration.  Which is to say, what happens if the list
> is empty.  Well, either memb is uninitialized, or there is an
> intialization somewhere upstream.  Declaration is not far before
> that loop, and it is
>         struct dlm_member *memb = NULL;
> Er...  So for that change to be correct you need to show that
> the list (ls->ls_nodes) can not be empty here.  Unfortunately,
> it looks like it very much can be empty, seeing that this
> is apparently the only place where elements are added to
> the list in question.  So on the very first call it will
> hit your "impossible to hit" case.  Which leads to...
> 
> * the patch had apparently never been tested.

Looking at the uses of ->ls_nodes,  I wonder if xarray would be
a better fit here.  Might be interesting to investigate...