Treacherous Variable Names

Now for the chronicle of a bug clearly caused by poor variable names. (And arguably also the lack of semantic meaning given to string types.) This  buggy function searches for a device with a given uuid:

/*
 * Search the specified glob for devices; return error code. On
 * success dev->fd is set to a valid file descriptor and the file
 * is locked.
 */
static int __find_dev(struct vdev *dev, struct uuid *uuid,
                      const char *const path)
{
	glob_t paths;
	int ret;
	size_t i;

	ret = glob(path, 0, NULL, &paths);
	/* Error handling omitted... */

	for (i = 0; i < paths.gl_pathc; i++) {
		const char *const fname = paths.gl_pathv[i];

		dev->fd = mopen(fname, O_RDWR);
		if (dev->fd < 0)
			continue;

		if (is_requested_device(dev->fd)) {
			ret = __try_dev(dev, uuid, path);
			if (!ret)
				goto found;
		}

		close(dev->fd);
	}

	ret = -ENOENT;

found:
	globfree(&paths);

	return ret;
}

mopen() is given fname, the path produced by glob(), but __try_dev() is given path, which is actually a glob. This caused the device to import correctly but report as its path the glob used to find it instead of its actual path. path is now named search_glob, and fname is now named path. Our existing tests did check that the device path was reported correctly, but they didn’t catch this because they all explicitly specified the exact path to the device to use, so the search glob was the correct path. If the search glob had been some kind of glob type instead of a string then the patch that caused this bug wouldn’t have compiled.