Re: [PATCH v2] grep: remove --mmap

[Eric Blake]

On 03/22/2010 03:52 AM, Paolo Bonzini wrote:
> mmap is a bad idea for sequentially accessed file because it will cause
> a page fault for every read page.  Just consider it a failed experiment,
> and ignore --mmap while accepting it for backwards compatibility.

Seems reasonable.  However, should we be experimenting with using
posix_fadvise instead, as that may have more beneficial behavior given
our sequential access patterns?

> @@ -320,7 +315,9 @@ static struct option const long_options[] =
>    {"line-number", no_argument, NULL, 'n'},
>    {"line-regexp", no_argument, NULL, 'x'},
>    {"max-count", required_argument, NULL, 'm'},
> -  {"mmap", no_argument, &mmap_option, 1},
> +
> +  /* FIXME: disabled in Mar 2010; warn towards end of 2011; remove in 2013.  
> */
> +  {"mmap", no_argument, NULL, MMAP_OPTION},
>    {"no-filename", no_argument, NULL, 'h'},

The spacing here makes it look like not only --mmap, but also
--no-filename and following are deprecated.  Is that worth tweaking in
the final commit?

> @@ -1474,7 +1412,7 @@ Miscellaneous:\n\
>    -v, --invert-match        select non-matching lines\n\
>    -V, --version             print version information and exit\n\
>        --help                display this help and exit\n\
> -      --mmap                use memory-mapped input if possible\n"));
> +      --mmap                ignored for backwards compatibility\n"));

Is it worth documenting something that will go away, or should we make
--help output shorter by omitting the line altogether?  (The info should
still document it until it is completely gone in 2013).

-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature