Subject: Re: df and multiple-mounted mountpoints (redux) From: Paul Eggert Date: Thu, 29 Apr 2004 12:49:49 -0700 To: Christian Jones CC: bug-coreutils@gnu.org It seems to me that the basic problem is that when there are multiple filesystems mounted on the same mount point, "df" prefers the first one listed, but on your host (and I think this is traditional behavior) it should prefer the last. Here's a proposed patch to implement this behavior. Can you please try this out? 2004-04-29 Paul Eggert * src/df.c (show_disk, show_point): If several filesystems are mounted on the same mount point, prefer the last one, not the first. Problem reported by Christian Jones in . (show_disk): Remove unused statp arg. Return bool, not int. (show_point): Rewrite to avoid gotos. Use the same algorithm for lofs and dummies for each pass through the mount table, rather than subtly different algorithms (which are probably inadvertent). Index: df.c =================================================================== RCS file: /home/meyering/coreutils/cu/src/df.c,v retrieving revision 1.157 diff -p -u -r1.157 df.c --- df.c 21 Jan 2004 22:55:51 -0000 1.157 +++ df.c 29 Apr 2004 19:34:02 -0000 @@ -521,22 +521,26 @@ done: } /* If DISK corresponds to a mount point, show its usage - and return nonzero. Otherwise, return zero. - STATP must be the result of `stat (DISK, STATP)'. */ -static int -show_disk (const char *disk, const struct stat *statp) + and return true. Otherwise, return false. */ +static bool +show_disk (char const *disk) { - struct mount_entry *me; + struct mount_entry const *me; + struct mount_entry const *best_match = NULL; for (me = mount_list; me; me = me->me_next) if (STREQ (disk, me->me_devname)) - { - show_dev (me->me_devname, me->me_mountdir, me->me_type, - me->me_dummy, me->me_remote); - return 1; - } + best_match = me; + + if (best_match) + { + show_dev (best_match->me_devname, best_match->me_mountdir, + best_match->me_type, best_match->me_dummy, + best_match->me_remote); + return true; + } - return 0; + return false; } /* Figure out which device file or directory POINT is mounted on @@ -547,121 +551,104 @@ show_point (const char *point, const str { struct stat disk_stats; struct mount_entry *me; - struct mount_entry *matching_dummy = NULL; + struct mount_entry const *best_match = NULL; /* If POINT is an absolute path name, see if we can find the mount point without performing any extra stat calls at all. */ if (*point == '/') { - for (me = mount_list; me; me = me->me_next) - { - if (STREQ (me->me_mountdir, point) && !STREQ (me->me_type, "lofs")) - { - /* Prefer non-dummy entries. */ - if (! me->me_dummy) - goto show_me; - matching_dummy = me; - } - } + /* Find the best match: prefer non-dummies, and then prefer the + last match if there are ties. */ - if (matching_dummy) - goto show_matching_dummy; + for (me = mount_list; me; me = me->me_next) + if (STREQ (me->me_mountdir, point) && !STREQ (me->me_type, "lofs") + && (!best_match || best_match->me_dummy || !me->me_dummy)) + best_match = me; } /* Calculate the real absolute path for POINT, and use that to find the mount point. This avoids statting unavailable mount points, which can hang df. */ - { - char *resolved = canonicalize_file_name (point); - ssize_t resolved_len = resolved ? strlen (resolved) : -1; - struct mount_entry *best_match = NULL; + if (! best_match) + { + char *resolved = canonicalize_file_name (point); + ssize_t resolved_len = resolved ? strlen (resolved) : -1; - if (1 <= resolved_len && resolved[0] == '/') - { - size_t best_match_len = 0; + if (1 <= resolved_len && resolved[0] == '/') + { + size_t best_match_len = 0; - for (me = mount_list; me; me = me->me_next) - if (! me->me_dummy) - { - size_t len = strlen (me->me_mountdir); - if (best_match_len < len && len <= resolved_len - && (len == 1 /* root file system */ - || ((len == resolved_len || resolved[len] == '/') - && strncmp (me->me_mountdir, resolved, len) == 0))) - { - best_match = me; - best_match_len = len; - } - } - } + for (me = mount_list; me; me = me->me_next) + if (!STREQ (me->me_type, "lofs") + && (!best_match || best_match->me_dummy || !me->me_dummy)) + { + size_t len = strlen (me->me_mountdir); + if (best_match_len <= len && len <= resolved_len + && (len == 1 /* root file system */ + || ((len == resolved_len || resolved[len] == '/') + && strncmp (me->me_mountdir, resolved, len) == 0))) + { + best_match = me; + best_match_len = len; + } + } + } - if (resolved) - free (resolved); + if (resolved) + free (resolved); - if (best_match && !STREQ (best_match->me_type, "lofs") - && stat (best_match->me_mountdir, &disk_stats) == 0 - && disk_stats.st_dev == statp->st_dev) + if (best_match + && (stat (best_match->me_mountdir, &disk_stats) != 0 + || disk_stats.st_dev != statp->st_dev)) + best_match = NULL; + } + + if (! best_match) + for (me = mount_list; me; me = me->me_next) { - me = best_match; - goto show_me; + if (me->me_dev == (dev_t) -1) + { + if (stat (me->me_mountdir, &disk_stats) == 0) + me->me_dev = disk_stats.st_dev; + else + { + error (0, errno, "%s", quote (me->me_mountdir)); + exit_status = 1; + /* So we won't try and fail repeatedly. */ + me->me_dev = (dev_t) -2; + } + } + + if (statp->st_dev == me->me_dev + && !STREQ (me->me_type, "lofs") + && (!best_match || best_match->me_dummy || !me->me_dummy)) + { + /* Skip bogus mtab entries. */ + if (stat (me->me_mountdir, &disk_stats) != 0 + || disk_stats.st_dev != me->me_dev) + me->me_dev = (dev_t) -2; + else + best_match = me; + } } - } - for (me = mount_list; me; me = me->me_next) + if (best_match) + show_dev (best_match->me_devname, best_match->me_mountdir, + best_match->me_type, best_match->me_dummy, best_match->me_remote); + else { - if (me->me_dev == (dev_t) -1) + /* We couldn't find the mount entry corresponding to POINT. Go ahead and + print as much info as we can; methods that require the device to be + present will fail at a later point. */ + + /* Find the actual mount point. */ + char *mp = find_mount_point (point, statp); + if (mp) { - if (stat (me->me_mountdir, &disk_stats) == 0) - me->me_dev = disk_stats.st_dev; - else - { - error (0, errno, "%s", quote (me->me_mountdir)); - exit_status = 1; - /* So we won't try and fail repeatedly. */ - me->me_dev = (dev_t) -2; - } - } - - if (statp->st_dev == me->me_dev) - { - /* Skip bogus mtab entries. */ - if (stat (me->me_mountdir, &disk_stats) != 0 - || disk_stats.st_dev != me->me_dev) - { - me->me_dev = (dev_t) -2; - continue; - } - - /* Prefer non-dummy entries. */ - if (! me->me_dummy) - goto show_me; - matching_dummy = me; + show_dev (0, mp, 0, 0, 0); + free (mp); } } - - if (matching_dummy) - goto show_matching_dummy; - - /* We couldn't find the mount entry corresponding to POINT. Go ahead and - print as much info as we can; methods that require the device to be - present will fail at a later point. */ - { - /* Find the actual mount point. */ - char *mp = find_mount_point (point, statp); - if (mp) - { - show_dev (0, mp, 0, 0, 0); - free (mp); - } - } - - return; - - show_matching_dummy: - me = matching_dummy; - show_me: - show_dev (me->me_devname, me->me_mountdir, me->me_type, me->me_dummy, - me->me_remote); } /* Determine what kind of node PATH is and show the disk usage @@ -671,7 +658,7 @@ static void show_entry (const char *path, const struct stat *statp) { if ((S_ISBLK (statp->st_mode) || S_ISCHR (statp->st_mode)) - && show_disk (path, statp)) + && show_disk (path)) return; show_point (path, statp);