Re: [PATCH 1/2] xfs: ensure tmp_logflags is initialized in xfs_bmap_del_extent_real

[Darrick J. Wong]

On Tue, Nov 28, 2023 at 12:19:23AM -0800, Christoph Hellwig wrote:
> On Tue, Nov 28, 2023 at 01:32:01PM +0800, Jiachen Zhang wrote:
> > In the case of returning -ENOSPC, ensure tmp_logflags is initialized by 0.
> > Otherwise the caller __xfs_bunmapi will set uninitialized illegal
> > tmp_logflags value into xfs log, which might cause unpredictable error
> > in the log recovery procedure.
> 
> This looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> 
> But I wonder if removing the local flags variable and always directly
> assigning to *logflagsp might be more robust in the long run.

Yes, I think it's better to eliminate opportunities for subtle logic
bombs by not open-coding variable aliasing.  Perhaps this function
should set *logflagsp = 0 at the start of the function so that we don't
have to deal with uninitialized outparams, especially since the caller
uses it even on an error return.

--D