[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: rm: don't waste time preprocessing on tmpfs or nfs
From: |
Jim Meyering |
Subject: |
Re: rm: don't waste time preprocessing on tmpfs or nfs |
Date: |
Wed, 01 Oct 2008 12:28:23 +0200 |
Andreas Schwab <address@hidden> wrote:
> Jim Meyering <address@hidden> writes:
>> diff --git a/src/remove.c b/src/remove.c
>> index ea52843..6c1eaea 100644
>> --- a/src/remove.c
>> +++ b/src/remove.c
>> @@ -1277,6 +1277,45 @@ dirent_count (struct stat const *st)
>> }
>> #endif /* HAVE_STRUCT_DIRENT_D_TYPE */
>>
>> +#if defined HAVE_SYS_VFS_H && HAVE_FSTATFS && HAVE_STRUCT_STATFS_F_TYPE
>> +# include <sys/statfs.h>
>> +# include "fs.h"
>> +/* FIXME: what about when f_type is not an integral type?
>> + deal with that if/when it's encountered. */
>> +static bool
>> +fs_handles_readdir_ordered_dirents_efficiently (uintmax_t fs_type)
>> +{
>> + switch (fs_type)
>> + {
>> + case S_MAGIC_TMPFS:
>> + case S_MAGIC_NFS:
>> + return true;
>
> Does this always do correct sign extension on the f_type constants?
Ha. You know the answer ;-)
Thanks.
I'll probably be using this change:
>From ebb424314a577c0b8ccaf3ef79c3c9a3379b31a5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 1 Oct 2008 12:21:58 +0200
Subject: [PATCH] remove.c: combine two helper functions, to avoid sign-extension
* src/remove.c (fs_handles_readdir_ordered_dirents_efficiently):
Remove function, so as not to have to worry about the type of
statfs.f_type and sign extension.
(dirent_inode_sort_may_be_useful): Adjust comment.
Perform the switch directly on the struct.member here, instead.
Andreas Schwab spotted the potential for a sign-extension bug,
that happens not to bite for the S_* f_type values currently used.
---
src/remove.c | 43 ++++++++++++++++++++++---------------------
1 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/src/remove.c b/src/remove.c
index 6c1eaea..30931b0 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -1280,24 +1280,11 @@ dirent_count (struct stat const *st)
#if defined HAVE_SYS_VFS_H && HAVE_FSTATFS && HAVE_STRUCT_STATFS_F_TYPE
# include <sys/statfs.h>
# include "fs.h"
-/* FIXME: what about when f_type is not an integral type?
- deal with that if/when it's encountered. */
-static bool
-fs_handles_readdir_ordered_dirents_efficiently (uintmax_t fs_type)
-{
- switch (fs_type)
- {
- case S_MAGIC_TMPFS:
- case S_MAGIC_NFS:
- return true;
- default:
- return false;
- }
-}
-/* Return true if it is easy to determine the file system type of the
- directory on which DIR_FD is open, and sorting dirents on inode numbers
- is known to improve traversal performance with that type of file system. */
+/* Return false if it is easy to determine the file system type of
+ the directory on which DIR_FD is open, and sorting dirents on
+ inode numbers is known not to improve traversal performance with
+ that type of file system. Otherwise, return true. */
static bool
dirent_inode_sort_may_be_useful (int dir_fd)
{
@@ -1307,10 +1294,24 @@ dirent_inode_sort_may_be_useful (int dir_fd)
while the cost of *not* performing it can be O(N^2) with
a very large constant. */
struct statfs fs_buf;
- bool skip = (fstatfs (dir_fd, &fs_buf) == 0
- && fs_handles_readdir_ordered_dirents_efficiently
- (fs_buf.f_type));
- return !skip;
+
+ /* If fstatfs fails, assume sorting would be useful. */
+ if (fstatfs (dir_fd, &fs_buf) != 0)
+ return true;
+
+ /* FIXME: what about when f_type is not an integral type?
+ deal with that if/when it's encountered. */
+ switch (fs_buf.f_type)
+ {
+ case S_MAGIC_TMPFS:
+ case S_MAGIC_NFS:
+ /* On a file system of any of these types, sorting
+ is unnecessary, and hence wasteful. */
+ return false;
+
+ default:
+ return true;
+ }
}
#else
static bool dirent_inode_sort_may_be_useful (int dir_fd) { return true; }
--
1.6.0.2.307.gc427