Re: [PATCH v2 1/7] Introduce read_entire_file

[Andrew Burgess <andrew.burgess@embecosm.com>]

* 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
>