On Fri, May 17, 2019 at 09:10:49AM -0500, Eric Sandeen wrote: > On 5/17/19 2:31 AM, Christoph Hellwig wrote: > > Just pass a straight bool aborted instead of abusing XFS_LI_ABORTED as a > > flag in function parameters. > > ... > > > out_abort: > > - xlog_cil_committed(ctx, XFS_LI_ABORTED); > > + xlog_cil_committed(ctx, true); > > return -EIO; > > Technically fine but I'm kind of on the fence about changes like this; > doesn't it make the code less readable? Passing a self-documenting > flag makes code reading a lot easier than seeing "true" and having > to cross-reference what the bool means. What's your thought on how this > helps? Is it worth keeping things like this more self-documenting? I hate this one because it passes a flag that is used for something entirely different and then actually interpreted as an int in boolean context later on. Switching to a proper bool seems like the simplest cleanup, but we could also add a different self describing flag if it bothers you. But it doesn't really seem like we'd ever grow another flag here.
