From patchwork Tue Apr 19 16:15:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 11806 Received: (qmail 81764 invoked by alias); 19 Apr 2016 16:15:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 81618 invoked by uid 89); 19 Apr 2016 16:15:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=cares, obscure, UD:st_mode, s_isdir X-HELO: mail-pa0-f44.google.com Received: from mail-pa0-f44.google.com (HELO mail-pa0-f44.google.com) (209.85.220.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 19 Apr 2016 16:15:19 +0000 Received: by mail-pa0-f44.google.com with SMTP id er2so8036306pad.3 for ; Tue, 19 Apr 2016 09:15:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=jzSH4t8OnfmIHTJST34SVtIKaQxDCyrXesDlz6cYhtM=; b=gNvjzPNgpp2QG3KW5fKZ+bbVU+0ULzvQK43vIiZiK598LAeT/qfnbFcSSRXUQyhNh6 YECPF/BhkqAzpggKEAVfAgQJb8Qtc/P3F7rzd5wXGXQqyD92IL4IqYoA2PKmZ5Y/p3aL r8gwaZLMEEl9tsQMfFnLYvlp4CibBNw7hqqR1pu6B+JV9pnHLGjnYMWK/TmGPP0Z8K9m sInsUKzzeKK8aR5saS9xWTZfMbWdR406wnlrKiUdVBnEn/fpR6wm5aIiluxA4CL55msu B1ZKlnUqdnmJDBE8TjOCeZqLgAqLXq5MD0m9OtRQd37Zt7bPB3AFyBiwBYACJh3CSmHL pooQ== X-Gm-Message-State: AOPr4FU8DronaheQYd9eJ3iQEpCpKci0vZ1ig0GY2mulI5nePNtTzuvD/fpRwWfGMCVPig== X-Received: by 10.66.48.164 with SMTP id m4mr5244726pan.57.1461082517247; Tue, 19 Apr 2016 09:15:17 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by smtp.gmail.com with ESMTPSA id k27sm25702298pfj.54.2016.04.19.09.15.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 Apr 2016 09:15:16 -0700 (PDT) From: Doug Evans To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] [PR symtab/17911] Recognize bad file types References: <560AA051.6060803@redhat.com> Date: Tue, 19 Apr 2016 09:15:15 -0700 In-Reply-To: <560AA051.6060803@redhat.com> (Pedro Alves's message of "Tue, 29 Sep 2015 15:29:37 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Pedro Alves writes: > On 09/19/2015 11:40 PM, Doug Evans wrote: > >> diff --git a/gdb/source.c b/gdb/source.c >> index fab974c..34384fa 100644 >> --- a/gdb/source.c >> +++ b/gdb/source.c >> @@ -684,7 +684,10 @@ source_info (char *ignore, int from_tty) >> } >> >> >> -/* Return True if the file NAME exists and is a regular file. */ >> +/* Return True if the file NAME exists and is a regular file. >> + If the result is false then errno is set to a useful value assuming we're >> + expecting a regular file. */ >> + >> static int >> is_regular_file (const char *name) >> { >> @@ -699,7 +702,14 @@ is_regular_file (const char *name) >> if (status != 0) >> return (errno != ENOENT); >> >> - return S_ISREG (st.st_mode); >> + if (S_ISREG (st.st_mode)) >> + return 1; >> + >> + if (S_ISDIR (st.st_mode)) >> + errno = EISDIR; >> + else >> + errno = EINVAL; >> + return 0; >> } >> >> /* Open a file named STRING, searching path PATH (dir names sep by some char) >> @@ -785,6 +795,7 @@ openp (const char *path, int opts, const char *string, >> { >> filename = NULL; >> fd = -1; >> + /* Note: is_regular_file will have set errno appropriately. */ >> } >> >> if (!(opts & OPF_SEARCH_IN_PATH)) >> @@ -808,6 +819,7 @@ openp (const char *path, int opts, const char *string, >> alloclen = strlen (path) + strlen (string) + 2; >> filename = alloca (alloclen); >> fd = -1; >> + errno = ENOENT; >> >> dir_vec = dirnames_to_char_ptr_vec (path); >> back_to = make_cleanup_free_char_ptr_vec (dir_vec); >> @@ -879,6 +891,10 @@ openp (const char *path, int opts, const char *string, >> if (fd >= 0) >> break; >> } >> + else >> + { >> + /* Note: is_regular_file will have set errno appropriately. */ >> + } >> } > > Seems like there are function calls after these that may > clobber errno. I think it'd be safer to do the usual > save_errno = errno; / errno = save_errno; dance. > >> +# Test passing bad files to gdb. PR symtab/17911 >> + >> +# We watch for specific text for errno, so only test on systems we have >> +# confidence in the error text. >> + >> +if { ! [ishost "*-*-linux*"] } { >> + return 0 >> +} > > Same comment as to the other patch -- I think we should instead > remove this check and if the error message is different on other > hosts, then whoever cares about such hosts will adjust the > test. I don't think there will be that many cases of > different messages. > > Otherwise LGTM. Here is what I committed. [This version passes errno back as an argument.] 2016-04-19 Doug Evans * source.c (is_regular_file): New arg errno_ptr. All callers updated. testsuite/ * gdb.base/bad-file.exp: New file. diff --git a/gdb/source.c b/gdb/source.c index a1f55b2..08cb2d3 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -685,9 +685,12 @@ source_info (char *ignore, int from_tty) } -/* Return True if the file NAME exists and is a regular file. */ +/* Return True if the file NAME exists and is a regular file. + If the result is false then *ERRNO_PTR is set to a useful value assuming + we're expecting a regular file. */ + static int -is_regular_file (const char *name) +is_regular_file (const char *name, int *errno_ptr) { struct stat st; const int status = stat (name, &st); @@ -698,9 +701,21 @@ is_regular_file (const char *name) on obscure systems where stat does not work as expected. */ if (status != 0) - return (errno != ENOENT); + { + if (errno != ENOENT) + return 1; + *errno_ptr = ENOENT; + return 0; + } - return S_ISREG (st.st_mode); + if (S_ISREG (st.st_mode)) + return 1; + + if (S_ISDIR (st.st_mode)) + *errno_ptr = EISDIR; + else + *errno_ptr = EINVAL; + return 0; } /* Open a file named STRING, searching path PATH (dir names sep by some char) @@ -775,22 +790,23 @@ openp (const char *path, int opts, const char *string, if ((opts & OPF_TRY_CWD_FIRST) || IS_ABSOLUTE_PATH (string)) { - int i; + int i, reg_file_errno; - if (is_regular_file (string)) + if (is_regular_file (string, ®_file_errno)) { filename = (char *) alloca (strlen (string) + 1); strcpy (filename, string); fd = gdb_open_cloexec (filename, mode, 0); if (fd >= 0) goto done; + last_errno = errno; } else { filename = NULL; fd = -1; + last_errno = reg_file_errno; } - last_errno = errno; if (!(opts & OPF_SEARCH_IN_PATH)) for (i = 0; string[i]; i++) @@ -821,6 +837,7 @@ openp (const char *path, int opts, const char *string, for (ix = 0; VEC_iterate (char_ptr, dir_vec, ix, dir); ++ix) { size_t len = strlen (dir); + int reg_file_errno; if (strcmp (dir, "$cwd") == 0) { @@ -879,13 +896,15 @@ openp (const char *path, int opts, const char *string, strcat (filename + len, SLASH_STRING); strcat (filename, string); - if (is_regular_file (filename)) + if (is_regular_file (filename, ®_file_errno)) { fd = gdb_open_cloexec (filename, mode, 0); if (fd >= 0) break; last_errno = errno; } + else + last_errno = reg_file_errno; } do_cleanups (back_to); diff --git a/gdb/testsuite/gdb.base/bad-file.exp b/gdb/testsuite/gdb.base/bad-file.exp new file mode 100644 index 0000000..82ca3d4 --- /dev/null +++ b/gdb/testsuite/gdb.base/bad-file.exp @@ -0,0 +1,54 @@ +# Copyright 2016 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . */ + +# Test passing bad files to gdb. PR symtab/17911 + +# Note: While we test for specific text in error messages, +# thus perhaps making the test host specific, if your host +# print different text then the plan is to update the expected text +# instead of making this test linux-only or some such. + +# There is no such file, but we still use the normal mechanism to pick +# its name and path. +standard_testfile + +gdb_exit +gdb_start + +set test "non-existent file" +set bad_file $testfile +remote_file host delete $bad_file +gdb_test_multiple "file $bad_file" "$test" { + -re "No such file or directory.\[\r\n\]+$gdb_prompt $" { + pass $test + } +} + +set test "directory" +set bad_file [standard_output_file {}] +remote_exec host "mkdir -p $bad_file" +gdb_test_multiple "file $bad_file" "$test" { + -re "Is a directory.\[\r\n\]+$gdb_prompt $" { + pass $test + } +} + +set test "neither file nor directory" +set bad_file "/dev/null" +gdb_test_multiple "file $bad_file" "$test" { + -re "Invalid argument.\[\r\n\]+$gdb_prompt $" { + pass $test + } +}