Discussion:
[PATCH] Return scoped_fd from open_source_file and find_and_open_source
Tom Tromey
2018-11-04 16:05:56 UTC
Permalink
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 <***@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(-)

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;
}
--
2.17.1
Simon Marchi
2018-11-09 21:58:57 UTC
Permalink
Post by Tom Tromey
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 th
Loading...