Return scoped_fd from open_source_file and find_and_open_source

Message ID 20181104160556.7619-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Nov. 4, 2018, 4:05 p.m. UTC
  This changes open_source_file and find_and_open_source to return
scoped_fd, then updates the callers as appropriate, including using
scoped_fd::to_file.

Tested by the buildbot.

gdb/ChangeLog
2018-11-04  Tom Tromey  <tom@tromey.com>

	* common/scoped_fd.h (class scoped_fd): Add move constructor and
	move assignment operator.
	* psymtab.c (psymtab_to_fullname): Update.
	* source.h (open_source_file): Return scoped_fd.
	(find_and_open_source): Likewise.
	* source.c (open_source_file): Return scoped_fd.
	(get_filename_and_charpos): Update.
	(print_source_lines_base): Update.  Use scoped_fd::to_file.
	(forward_search_command): Likewise.
	(reverse_search_command): Likewise.
	(find_and_open_source): Return scoped_fd.
	* tui/tui-source.c (tui_set_source_content): Update.  Use
	gdb_file_up.
---
 gdb/ChangeLog          | 16 +++++++++++
 gdb/common/scoped_fd.h | 19 +++++++++++++
 gdb/psymtab.c          |  7 ++---
 gdb/source.c           | 63 ++++++++++++++++++++----------------------
 gdb/source.h           | 10 ++++---
 gdb/tui/tui-source.c   | 47 ++++++++++++++-----------------
 6 files changed, 94 insertions(+), 68 deletions(-)
  

Comments

Simon Marchi Nov. 9, 2018, 9:58 p.m. UTC | #1
On 2018-11-04 11:05 a.m., Tom Tromey wrote:
> This changes open_source_file and find_and_open_source to return

> scoped_fd, then updates the callers as appropriate, including using

> scoped_fd::to_file.

> 

> Tested by the buildbot.


LGTM, thanks!

Simon
  

Patch

diff --git a/gdb/common/scoped_fd.h b/gdb/common/scoped_fd.h
index d20e18a2c0..99fcfacaf9 100644
--- a/gdb/common/scoped_fd.h
+++ b/gdb/common/scoped_fd.h
@@ -29,12 +29,31 @@  class scoped_fd
 {
 public:
   explicit scoped_fd (int fd = -1) noexcept : m_fd (fd) {}
+
+  scoped_fd (scoped_fd &&other)
+    : m_fd (other.m_fd)
+  {
+    other.m_fd = -1;
+  }
+
   ~scoped_fd ()
   {
     if (m_fd >= 0)
       close (m_fd);
   }
 
+  scoped_fd &operator= (scoped_fd &&other)
+  {
+    if (m_fd != other.m_fd)
+      {
+	if (m_fd >= 0)
+	  close (m_fd);
+	m_fd = other.m_fd;
+	other.m_fd = -1;
+      }
+    return *this;
+  }
+
   DISABLE_COPY_AND_ASSIGN (scoped_fd);
 
   int release () noexcept
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 915e4fb582..6d76e8d489 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1129,12 +1129,11 @@  psymtab_to_fullname (struct partial_symtab *ps)
   if (ps->fullname == NULL)
     {
       gdb::unique_xmalloc_ptr<char> fullname;
-      int fd = find_and_open_source (ps->filename, ps->dirname, &fullname);
+      scoped_fd fd = find_and_open_source (ps->filename, ps->dirname,
+					   &fullname);
       ps->fullname = fullname.release ();
 
-      if (fd >= 0)
-	close (fd);
-      else
+      if (fd.get () < 0)
 	{
 	  /* rewrite_source_path would be applied by find_and_open_source, we
 	     should report the pathname where GDB tried to find the file.  */
diff --git a/gdb/source.c b/gdb/source.c
index ec0ea3b81e..126591f1ee 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -969,7 +969,9 @@  rewrite_source_path (const char *path)
   return gdb::unique_xmalloc_ptr<char> (new_path);
 }
 
-int
+/* See source.h.  */
+
+scoped_fd
 find_and_open_source (const char *filename,
 		      const char *dirname,
 		      gdb::unique_xmalloc_ptr<char> *fullname)
@@ -995,7 +997,7 @@  find_and_open_source (const char *filename,
       if (result >= 0)
 	{
 	  *fullname = gdb_realpath (fullname->get ());
-	  return result;
+	  return scoped_fd (result);
 	}
 
       /* Didn't work -- free old one, try again.  */
@@ -1056,7 +1058,7 @@  find_and_open_source (const char *filename,
 			OPEN_MODE, fullname);
     }
 
-  return result;
+  return scoped_fd (result);
 }
 
 /* Open a source file given a symtab S.  Returns a file descriptor or
@@ -1064,14 +1066,15 @@  find_and_open_source (const char *filename,
    
    This function is a convience function to find_and_open_source.  */
 
-int
+scoped_fd
 open_source_file (struct symtab *s)
 {
   if (!s)
-    return -1;
+    return scoped_fd (-1);
 
   gdb::unique_xmalloc_ptr<char> fullname;
-  int fd = find_and_open_source (s->filename, SYMTAB_DIRNAME (s), &fullname);
+  scoped_fd fd = find_and_open_source (s->filename, SYMTAB_DIRNAME (s),
+				       &fullname);
   s->fullname = fullname.release ();
   return fd;
 }
@@ -1093,11 +1096,9 @@  symtab_to_fullname (struct symtab *s)
      to handle cases like the file being moved.  */
   if (s->fullname == NULL)
     {
-      int fd = open_source_file (s);
+      scoped_fd fd = open_source_file (s);
 
-      if (fd >= 0)
-	close (fd);
-      else
+      if (fd.get () < 0)
 	{
 	  gdb::unique_xmalloc_ptr<char> fullname;
 
@@ -1216,7 +1217,7 @@  get_filename_and_charpos (struct symtab *s, char **fullname)
 {
   int linenums_changed = 0;
 
-  scoped_fd desc (open_source_file (s));
+  scoped_fd desc = open_source_file (s);
   if (desc.get () < 0)
     {
       if (fullname)
@@ -1270,7 +1271,7 @@  print_source_lines_base (struct symtab *s, int line, int stopline,
 			 print_source_lines_flags flags)
 {
   int c;
-  int desc;
+  scoped_fd desc;
   int noprint = 0;
   int nlines = stopline - line;
   struct ui_out *uiout = current_uiout;
@@ -1289,24 +1290,26 @@  print_source_lines_base (struct symtab *s, int line, int stopline,
 	{
 	  last_source_visited = s;
 	  desc = open_source_file (s);
+	  if (desc.get () < 0)
+	    {
+	      last_source_error = desc.get ();
+	      noprint = 1;
+	    }
 	}
       else
 	{
-	  desc = last_source_error;
 	  flags |= PRINT_SOURCE_LINES_NOERROR;
+	  noprint = 1;
 	}
     }
   else
     {
-      desc = last_source_error;
       flags |= PRINT_SOURCE_LINES_NOERROR;
       noprint = 1;
     }
 
-  if (desc < 0 || noprint)
+  if (noprint)
     {
-      last_source_error = desc;
-
       if (!(flags & PRINT_SOURCE_LINES_NOERROR))
 	{
 	  const char *filename = symtab_to_filename_for_display (s);
@@ -1350,22 +1353,16 @@  print_source_lines_base (struct symtab *s, int line, int stopline,
   last_source_error = 0;
 
   if (s->line_charpos == 0)
-    find_source_lines (s, desc);
+    find_source_lines (s, desc.get ());
 
   if (line < 1 || line > s->nlines)
-    {
-      close (desc);
-      error (_("Line number %d out of range; %s has %d lines."),
-	     line, symtab_to_filename_for_display (s), s->nlines);
-    }
+    error (_("Line number %d out of range; %s has %d lines."),
+	   line, symtab_to_filename_for_display (s), s->nlines);
 
-  if (lseek (desc, s->line_charpos[line - 1], 0) < 0)
-    {
-      close (desc);
-      perror_with_name (symtab_to_filename_for_display (s));
-    }
+  if (lseek (desc.get (), s->line_charpos[line - 1], 0) < 0)
+    perror_with_name (symtab_to_filename_for_display (s));
 
-  gdb_file_up stream (fdopen (desc, FDOPEN_MODE));
+  gdb_file_up stream = desc.to_file (FDOPEN_MODE);
   clearerr (stream.get ());
 
   while (nlines-- > 0)
@@ -1549,7 +1546,7 @@  forward_search_command (const char *regex, int from_tty)
   if (current_source_symtab == 0)
     select_source_symtab (0);
 
-  scoped_fd desc (open_source_file (current_source_symtab));
+  scoped_fd desc = open_source_file (current_source_symtab);
   if (desc.get () < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
 
@@ -1563,7 +1560,7 @@  forward_search_command (const char *regex, int from_tty)
       < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
 
-  gdb_file_up stream (fdopen (desc.release (), FDOPEN_MODE));
+  gdb_file_up stream = desc.to_file (FDOPEN_MODE);
   clearerr (stream.get ());
   while (1)
     {
@@ -1631,7 +1628,7 @@  reverse_search_command (const char *regex, int from_tty)
   if (current_source_symtab == 0)
     select_source_symtab (0);
 
-  scoped_fd desc (open_source_file (current_source_symtab));
+  scoped_fd desc = open_source_file (current_source_symtab);
   if (desc.get () < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
 
@@ -1645,7 +1642,7 @@  reverse_search_command (const char *regex, int from_tty)
       < 0)
     perror_with_name (symtab_to_filename_for_display (current_source_symtab));
 
-  gdb_file_up stream (fdopen (desc.release (), FDOPEN_MODE));
+  gdb_file_up stream = desc.to_file (FDOPEN_MODE);
   clearerr (stream.get ());
   while (line > 1)
     {
diff --git a/gdb/source.h b/gdb/source.h
index a8769506a0..9152ec0930 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -19,6 +19,8 @@ 
 #ifndef SOURCE_H
 #define SOURCE_H
 
+#include "common/scoped_fd.h"
+
 struct symtab;
 
 /* See openp function definition for their description.  */
@@ -66,13 +68,13 @@  extern void init_source_path (void);
    On Failure
      An invalid file descriptor is returned (the return value is negative).
      FULLNAME is set to NULL.  */
-extern int find_and_open_source (const char *filename,
-				 const char *dirname,
-				 gdb::unique_xmalloc_ptr<char> *fullname);
+extern scoped_fd find_and_open_source (const char *filename,
+				       const char *dirname,
+				       gdb::unique_xmalloc_ptr<char> *fullname);
 
 /* Open a source file given a symtab S.  Returns a file descriptor or
    negative number for error.  */
-extern int open_source_file (struct symtab *s);
+extern scoped_fd open_source_file (struct symtab *s);
 
 extern gdb::unique_xmalloc_ptr<char> rewrite_source_path (const char *path);
 
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index a26b7b0cbf..3c4f06b01a 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -46,8 +46,7 @@  tui_set_source_content (struct symtab *s,
 
   if (s != (struct symtab *) NULL)
     {
-      FILE *stream;
-      int i, desc, c, line_width, nlines;
+      int i, c, line_width, nlines;
       char *src_line = 0;
 
       if ((ret = tui_alloc_source_buffer (TUI_SRC_WIN)) == TUI_SUCCESS)
@@ -56,8 +55,8 @@  tui_set_source_content (struct symtab *s,
 	  /* Take hilite (window border) into account, when
 	     calculating the number of lines.  */
 	  nlines = (line_no + (TUI_SRC_WIN->generic.height - 2)) - line_no;
-	  desc = open_source_file (s);
-	  if (desc < 0)
+	  scoped_fd desc = open_source_file (s);
+	  if (desc.get () < 0)
 	    {
 	      if (!noerror)
 		{
@@ -72,22 +71,17 @@  tui_set_source_content (struct symtab *s,
 	  else
 	    {
 	      if (s->line_charpos == 0)
-		find_source_lines (s, desc);
+		find_source_lines (s, desc.get ());
 
 	      if (line_no < 1 || line_no > s->nlines)
-		{
-		  close (desc);
-		  printf_unfiltered ("Line number %d out of range; "
-				     "%s has %d lines.\n",
-				     line_no,
-				     symtab_to_filename_for_display (s),
-				     s->nlines);
-		}
-	      else if (lseek (desc, s->line_charpos[line_no - 1], 0) < 0)
-		{
-		  close (desc);
-		  perror_with_name (symtab_to_filename_for_display (s));
-		}
+		printf_unfiltered ("Line number %d out of range; "
+				   "%s has %d lines.\n",
+				   line_no,
+				   symtab_to_filename_for_display (s),
+				   s->nlines);
+	      else if (lseek (desc.get (), s->line_charpos[line_no - 1], 0)
+		       < 0)
+		perror_with_name (symtab_to_filename_for_display (s));
 	      else
 		{
 		  int offset, cur_line_no, cur_line, cur_len, threshold;
@@ -108,8 +102,8 @@  tui_set_source_content (struct symtab *s,
                      line and the offset to start the display.  */
 		  offset = src->horizontal_offset;
 		  threshold = (line_width - 1) + offset;
-		  stream = fdopen (desc, FOPEN_RT);
-		  clearerr (stream);
+		  gdb_file_up stream = desc.to_file (FOPEN_RT);
+		  clearerr (stream.get ());
 		  cur_line = 0;
 		  src->gdbarch = get_objfile_arch (SYMTAB_OBJFILE (s));
 		  src->start_line_or_addr.loa = LOA_LINE;
@@ -123,7 +117,7 @@  tui_set_source_content (struct symtab *s,
 			= TUI_SRC_WIN->generic.content[cur_line];
 
 		      /* Get the first character in the line.  */
-		      c = fgetc (stream);
+		      c = fgetc (stream.get ());
 
 		      if (offset == 0)
 			src_line = TUI_SRC_WIN->generic.content[cur_line]
@@ -200,21 +194,21 @@  tui_set_source_content (struct symtab *s,
 				{ /* If we have not reached EOL, then
 				     eat chars until we do.  */
 				  while (c != EOF && c != '\n' && c != '\r')
-				    c = fgetc (stream);
+				    c = fgetc (stream.get ());
 				  /* Handle non-'\n' end-of-line.  */
 				  if (c == '\r' 
-				      && (c = fgetc (stream)) != '\n' 
+				      && (c = fgetc (stream.get ())) != '\n'
 				      && c != EOF)
 				    {
-				       ungetc (c, stream);
-				       c = '\r';
+				      ungetc (c, stream.get ());
+				      c = '\r';
 				    }
 				  
 				}
 			    }
 			  while (c != EOF && c != '\n' && c != '\r' 
 				 && i < threshold 
-				 && (c = fgetc (stream)));
+				 && (c = fgetc (stream.get ())));
 			}
 		      /* Now copy the line taking the offset into
 			 account.  */
@@ -232,7 +226,6 @@  tui_set_source_content (struct symtab *s,
 		    }
 		  if (offset > 0)
 		    xfree (src_line);
-		  fclose (stream);
 		  TUI_SRC_WIN->generic.content_size = nlines;
 		  ret = TUI_SUCCESS;
 		}