Thanks for the feedback, Alan...
On Tue, 6 Jun 2023 18:33:22 -0400, Alan Stern wrote:
> On Wed, Jun 07, 2023 at 12:15:18AM +0200, David Disseldorp wrote:
> > fsg_lun_is_open() will always and only be true after a successful
> > fsg_lun_open() call, so drop the unnecessary conditional.
>
> I don't follow the reasoning. The relevant code is:
>
> if (cfg->filename) {
> rc = fsg_lun_open(lun, cfg->filename);
> if (rc)
> goto error_lun;
> }
> ...
> if (fsg_lun_is_open(lun)) {
>
> So at this point, the fsg_lun_is_open() condition is true iff
> cfg->filename was set upon entry. What makes this test unnecessary?
The fsg_lun_is_open() test is unnecessary as it will always be true
following a successful fsg_lun_open() call and will always be false if
cfg->filename is unset. This means that the logic from the
fsg_lun_is_open() conditional block can be appended directly after the
error_lun goto. pathbuf allocation ('...' above) is only needed for
the open case, so can also be wrapped into the conditional block.
I'd be happy to update the commit message if the explanation above
makes things clearer. I should probably also mention that I've tested
this using dummy-hcd.
Cheers, David
