Re: [PATCH v2 1/7] Introduce read_entire_file

Andrew Burgess
2020-10-02T10:25:07Z

* Tom Tromey <tom@tromey.com> [2020-06-23 07:20:00 -0600]:

> This adds a new function, read_entire_file, and changes the source
> cache to use it. This will be used in a subsequent patch as well.
>
> gdb/ChangeLog
> 2020-06-22 Tom Tromey <tom@tromey.com>
>
> * utils.h (myread): Don't declare.
> * utils.c (myread): Remove.
> * source-cache.c (source_cache::get_plain_source_lines): Use
> read_entire_file.
>
> gdbsupport/ChangeLog
> 2020-06-22 Tom Tromey <tom@tromey.com>
>
> * filestuff.h (read_entire_file): Declare.
> * filestuff.cc (read_entire_file): New function.

LGTM.

Thanks,
Andrew

> ---
> gdb/ChangeLog | 7 +++++++
> gdb/source-cache.c | 14 +++++---------
> gdb/utils.c | 22 ----------------------
> gdb/utils.h | 2 --
> gdbsupport/ChangeLog | 5 +++++
> gdbsupport/filestuff.cc | 33 +++++++++++++++++++++++++++++++++
> gdbsupport/filestuff.h | 9 +++++++++
> 7 files changed, 59 insertions(+), 33 deletions(-)
>
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 9196e3a19e3..63e08c3aeb0 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -18,6 +18,7 @@
>
> #include "defs.h"
> #include "source-cache.h"
> +#include "gdbsupport/filestuff.h"
> #include "gdbsupport/scoped_fd.h"
> #include "source.h"
> #include "cli/cli-style.h"
> @@ -56,14 +57,9 @@ source_cache::get_plain_source_lines (struct symtab *s,
> if (desc.get () < 0)
> perror_with_name (symtab_to_filename_for_display (s));
>
> - struct stat st;
> - if (fstat (desc.get (), &st) < 0)
> - perror_with_name (symtab_to_filename_for_display (s));
> -
> - std::string lines;
> - lines.resize (st.st_size);
> - if (myread (desc.get (), &lines[0], lines.size ()) < 0)
> - perror_with_name (symtab_to_filename_for_display (s));
> + time_t file_mtime;
> + std::string lines = read_entire_file (symtab_to_filename_for_display (s),
> + desc.get (), &file_mtime);
>
> time_t mtime = 0;
> if (SYMTAB_OBJFILE (s) != NULL && SYMTAB_OBJFILE (s)->obfd != NULL)
> @@ -71,7 +67,7 @@ source_cache::get_plain_source_lines (struct symtab *s,
> else if (exec_bfd)
> mtime = exec_bfd_mtime;
>
> - if (mtime && mtime < st.st_mtime)
> + if (mtime && mtime < file_mtime)
> warning (_("Source file is more recent than executable."));
>
> std::vector<off_t> offsets;
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 102db28787f..023adfae066 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -687,28 +687,6 @@ flush_streams ()
> gdb_stderr->flush ();
> }
>
> -/* My replacement for the read system call.
> - Used like `read' but keeps going if `read' returns too soon. */
> -
> -int
> -myread (int desc, char *addr, int len)
> -{
> - int val;
> - int orglen = len;
> -
> - while (len > 0)
> - {
> - val = read (desc, addr, len);
> - if (val < 0)
> - return val;
> - if (val == 0)
> - return orglen - len;
> - len -= val;
> - addr += val;
> - }
> - return orglen;
> -}
> -
> void
> print_spaces (int n, struct ui_file *file)
> {
> diff --git a/gdb/utils.h b/gdb/utils.h
> index 3434ff1caa2..d0989204128 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -540,8 +540,6 @@ void dummy_obstack_deallocate (void *object, void *data);
> extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout);
> #endif
>
> -extern int myread (int, char *, int);
> -
> /* Resource limits used by getrlimit and setrlimit. */
>
> enum resource_limit_kind
> diff --git a/gdbsupport/filestuff.cc b/gdbsupport/filestuff.cc
> index 179c4258918..0f0a238beb0 100644
> --- a/gdbsupport/filestuff.cc
> +++ b/gdbsupport/filestuff.cc
> @@ -501,3 +501,36 @@ mkdir_recursive (const char *dir)
> component_start = component_end;
> }
> }
> +
> +/* See filestuff.h. */
> +
> +std::string
> +read_entire_file (const char *name, int fd, time_t *mtime)
> +{
> + struct stat st;
> + if (fstat (fd, &st) < 0)
> + perror_with_name (name);
> + size_t len = st.st_size;
> +
> + std::string lines;
> + lines.resize (len);
> + char *addr = &lines[0];
> +
> + while (len > 0)
> + {
> + int val = read (fd, addr, len);
> + if (val < 0)
> + perror_with_name (name);
> + if (val == 0)
> + break;
> + len -= val;
> + addr += val;
> + }
> +
> + if (mtime != nullptr)
> + *mtime = st.st_mtime;
> +
> + /* Just in case we really did end up short. */
> + lines.resize (st.st_size - len);
> + return lines;
> +}
> diff --git a/gdbsupport/filestuff.h b/gdbsupport/filestuff.h
> index e36936a84ce..84924f462e2 100644
> --- a/gdbsupport/filestuff.h
> +++ b/gdbsupport/filestuff.h
> @@ -139,4 +139,13 @@ extern bool is_regular_file (const char *name, int *errno_ptr);
>
> extern bool mkdir_recursive (const char *dir);
>
> +/* Read and return the entire contents of a file. The file must
> + already be open on FD. NAME is the file name it is used when
> + reporting errors, which is done by throwing an exception. The
> + mtime of the file is optionally stored in *MTIME; if MTIME is
> + nullptr, this is ignored. */
> +
> +extern std::string read_entire_file (const char *name, int fd,
> + time_t *mtime = nullptr);
> +
> #endif /* COMMON_FILESTUFF_H */
> --
> 2.17.2
>