Comment 4 for bug 1640823

This felt unusually risk to me, so I took a look at the patch before releasing. I found a number of problems:

1. This patch leaks a file descriptor every time the new code path runs. loop_ctl_fd is opened but not closed.

2. "int devnr = -1" is defined but never used.

3.

> ll->ncur = ioctl(loop_ctl_fd, LOOP_CTL_GET_FREE);

This is wrong. ncur is an integer iterator, not necessarily a loop number. It keeps state between multiple calls to looplist_next, and you're clobbering it. It may be the case that it happens not to get used again when LLFLG_LOOPCTL is set, but that's no reason to re-use an existing variable for a different purpose. This makes the code much more difficult to check for unexpected side effects and thus may hide bugs.

4. looplist_next() is called to acquire both free and used loop device fds (based on LLFLG_USEDONLY/LLFLG_FREEONLY; see looplist_open_dev()). The fashion in which it does this is quite convulated. Additionally, it appears designed to be called multiple times (like a C++ iterator). The new code will always return a new loop device when requested. This is of course the point of this SRU. However, a consequence of this is that calls to looplist_next with LLFLG_FREEONLY are now unbounded, and this may have implications throughout the code. I can't find any, but this is certainly Regression Potential territory, and IMHO all known use cases that call looplist_next() should be checked for correct behaviour in SRU verification.

5. The "mount" and "umount" commands also call functions defined by lomount.c, so loop-affecting behaviour should be checked in these, too.

It feels like luck to me that this code didn't introduce a bug that I could find. However, we want to minimise regression risk in SRUs. Even though I couldn't find a specific bug, issues 1 and 3 above unnecessarily add to the risk of regression in this SRU, IMHO, and are easy to mitigate. So I'm marking this bug verification-failed. You might as well fix issue 2 while you're there. For issues 4 and 5, I'll update the test plan in the bug description.