From patchwork Wed Sep 27 14:22:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 76767 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A571E3870C1C for ; Wed, 27 Sep 2023 14:22:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A571E3870C1C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1695824574; bh=5Skbye1pbdleF3T0AL5mnoSwVxBekNvHYr5HKE9NXYw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=cL/jycMI2PW1B17AzOzNtTHW5T65iFcLSMUd0zBBfnlZtZpQwhNJnj7obuj1r4sat HNvhQJCTgcpBt9jbBKRS2rVM9qPuYUBtTJEuhkOHnBYHtIQe9DmXHOdJafeEUgJunM oQ5DMOtL9dNiz0o/CtHjklm8JkLXDqYKfo0iZNRo= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id D3A3B38618D0 for ; Wed, 27 Sep 2023 14:22:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D3A3B38618D0 Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-620-5E6keGmmMYqdtHFKpUEGAA-1; Wed, 27 Sep 2023 10:22:18 -0400 X-MC-Unique: 5E6keGmmMYqdtHFKpUEGAA-1 Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-65b0af1b0e2so124558676d6.0 for ; Wed, 27 Sep 2023 07:22:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695824538; x=1696429338; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5Skbye1pbdleF3T0AL5mnoSwVxBekNvHYr5HKE9NXYw=; b=AVpV/fkVbd37HYNQv/JJPTBDQWqEf5bHO9esDPz1D15W/wKQ6yWNoz86Nu5kkXOG2u dwOjj9Z0ebH53RyzMAU1JZmyzyYlM/1vaNBjGtTqR45hUIbalpkudsCQCRF9i2t7ulHv Ljc0mlUmNsXk05nv9vr7Z9pm78Y5KIad/D5vXPf9ojm7+Q9OPPVukXFTIUY5DhYeOg9V H5OdLPBlBm54lmQowV4PqbX6GzHTWmBg0P5CQMEWwjXGwnIooJLt1VMAXMSlYXbMCIag csb/tHnxUEa41ZObY4zyv/bchn5JEuuV8Fr8wu4qmCHXPK3EoSIn2ediOzrurvliP/7/ SLvg== X-Gm-Message-State: AOJu0YzZsI8q8OlqowDY9daKd4HE7YrJAnFSj1cP8a8xO1F9OaH3b3Xm u9kgjTurUDVZ93Ge9FUyBMj1FB1m1yV2wd7ytDJyyCcKJjchm4ydMptQjNijLH729+17/plYPE7 DjbTCI+DA+tIYl0zdyIh+734mYx8H0QIkQOQuWR2yb4K8oI/IOCh7AnBnQQex/SEZNnra58B9xW y/gez7Ag== X-Received: by 2002:a05:6214:b2b:b0:65c:fec5:6f0 with SMTP id w11-20020a0562140b2b00b0065cfec506f0mr1731719qvj.45.1695824537725; Wed, 27 Sep 2023 07:22:17 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFUJOEtA//T38ywQUhZ+MW6wz5XrtqXG9EfNYq37ml+OWPAXXDHKAmoVdAxVS8/GOfhTeRaLA== X-Received: by 2002:a05:6214:b2b:b0:65c:fec5:6f0 with SMTP id w11-20020a0562140b2b00b0065cfec506f0mr1731697qvj.45.1695824537285; Wed, 27 Sep 2023 07:22:17 -0700 (PDT) Received: from localhost ([31.111.84.209]) by smtp.gmail.com with ESMTPSA id t12-20020a05620a004c00b00767dcf6f4adsm3363471qkt.51.2023.09.27.07.22.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Sep 2023 07:22:17 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 4/5] gdb: remove print_sys_errmsg Date: Wed, 27 Sep 2023 15:22:04 +0100 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" This started with me running into this comment in symfile.c: /* FIXME, should use print_sys_errmsg but it's not filtered. */ gdb_printf (_("`%ps' has disappeared; keeping its symbols.\n"), styled_string (file_name_style.style (), filename)); In this particular case I think I disagree with the comment; I think the output should be a warning rather than just a message printed to gdb_stdout, I think when the executable, or some other objfile that is currently being debugged, disappears from disk, this is likely an unexpected situation, and worth warning the user about. So, in theory, I could just call print_sys_errmsg and remove the comment, but that would mean loosing the filename styling in the output... so in the end I remove the comment and updated the code to call warning. But that got me looking at print_sys_errmsg and how it's used. Currently the function takes a string and an errno, and prints, to stderr, the string followed by the result of calling strerror on the errno. In some places the string passed to print_sys_errmsg is just a filename, and this is used when something goes wrong. In these cases, I think calling warning rather than gdb_printf to gdb_stderr, would be better, and in fact, in a couple of places we manually print a "warning" prefix, and then call print_sys_errmsg. And so, for these users I have added a new function warning_filename_and_errno, which takes a filename, which is printed with styling, and an errno, which is passed through strerror and the resulting string printed. This new function calls warning to print its output. I then updated some of the print_sys_errmsg users to use this new function. Some other users of print_sys_errmsg are also emitting what is clearly a warning, however, the string being passed in is more than just a filename, so the new warning_filename_and_errno function can't be used, it would style the whole string. For these users I have switched to calling warning directly, this allows me to style the warning message correctly. Finally, in inflow.c there is one last call to print_sys_errmsg, in this case I just inlined the definition of print_sys_errmsg. This is a really weird case, as after printing this message GDB just does a hard exit. This is pretty old code, dating back to the initial GDB import, I guess it should be updated to call error() maybe, but I'm reluctant to make this change as part of this commit, just in case there's some reason why we can't throw an error at this point. With that done there are now no users of print_sys_errmsg, and so the old function can be removed. While I was doing all of the above I added some additional filename styling in soure.c, this is in an else block where the if contained the print_sys_errmsg call, so these felt related. And finally, while I was updating the uses of print_sys_errmsg in procfs.c, I noticed that we used a static errmsg buffer to format some error strings. As the above changes got rid of one of the users of errmsg I also removed the other two users, and the static buffer. --- gdb/inflow.c | 3 ++- gdb/main.c | 7 +------ gdb/procfs.c | 17 ++++++++++------- gdb/source.c | 15 ++++----------- gdb/symfile.c | 5 ++--- gdb/utils.c | 9 ++++----- gdb/utils.h | 8 +++++++- gdb/windows-nat.c | 2 +- 8 files changed, 31 insertions(+), 35 deletions(-) diff --git a/gdb/inflow.c b/gdb/inflow.c index 767cfd02c48..095c5f03672 100644 --- a/gdb/inflow.c +++ b/gdb/inflow.c @@ -766,7 +766,8 @@ check_syscall (const char *msg, int result) { if (result < 0) { - print_sys_errmsg (msg, errno); + gdb_printf (gdb_stderr, "%s:%s.\n", msg, + safe_strerror (errno)); _exit (1); } } diff --git a/gdb/main.c b/gdb/main.c index cf46f6acb20..1ca0fdeee1b 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -115,12 +115,7 @@ set_gdb_data_directory (const char *new_datadir) struct stat st; if (stat (new_datadir, &st) < 0) - { - int save_errno = errno; - - gdb_printf (gdb_stderr, "Warning: "); - print_sys_errmsg (new_datadir, save_errno); - } + warning_filename_and_errno (new_datadir, errno); else if (!S_ISDIR (st.st_mode)) warning (_("%ps is not a directory."), styled_string (file_name_style.style (), new_datadir)); diff --git a/gdb/procfs.c b/gdb/procfs.c index 706ccf0965c..48e9f3dd4b5 100644 --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -46,6 +46,7 @@ #include "gdbsupport/scoped_fd.h" #include "gdbsupport/pathstuff.h" #include "gdbsupport/buildargv.h" +#include "cli/cli-style.h" /* This module provides the interface between GDB and the /proc file system, which is used on many versions of Unix @@ -558,7 +559,7 @@ enum { NOKILL, KILL }; static void dead_procinfo (procinfo *pi, const char *msg, int kill_p) { - print_sys_errmsg (pi->pathname, errno); + warning_filename_and_errno (pi->pathname, errno); if (kill_p == KILL) kill (pi->pid, SIGKILL); @@ -594,18 +595,20 @@ static void proc_warn (procinfo *pi, const char *func, int line) { int saved_errno = errno; - std::string errmsg - = string_printf ("procfs: %s line %d, %s", func, line, pi->pathname); - print_sys_errmsg (errmsg.c_str (), saved_errno); + warning ("procfs: %s line %d, %ps: %s", + func, line, styled_string (file_name_style.style (), + pi->pathname), + safe_strerror (saved_errno)); } static void proc_error (procinfo *pi, const char *func, int line) { int saved_errno = errno; - std::string errmsg - = string_printf ("procfs: %s line %d, %s", func, line, pi->pathname); - perror_with_name (errmsg.c_str (), saved_errno); + error ("procfs: %s line %d, %ps: %s", + func, line, styled_string (file_name_style.style (), + pi->pathname), + safe_strerror (saved_errno)); } /* Updates the status struct in the procinfo. There is a 'valid' diff --git a/gdb/source.c b/gdb/source.c index 5bdd729be8b..f648adc4520 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -587,12 +587,7 @@ add_path (const char *dirname, char **which_path, int parse_separators) a directory/etc, then having them in the path should be harmless. */ if (stat (name, &st) < 0) - { - int save_errno = errno; - - gdb_printf (gdb_stderr, "Warning: "); - print_sys_errmsg (name, save_errno); - } + warning_filename_and_errno (name, errno); else if ((st.st_mode & S_IFMT) != S_IFDIR) warning (_("%ps is not a directory."), styled_string (file_name_style.style (), name)); @@ -1341,11 +1336,9 @@ print_source_lines_base (struct symtab *s, int line, int stopline, if (!(flags & PRINT_SOURCE_LINES_NOERROR)) { const char *filename = symtab_to_filename_for_display (s); - int len = strlen (filename) + 100; - char *name = (char *) alloca (len); - - xsnprintf (name, len, "%d\t%s", line, filename); - print_sys_errmsg (name, errcode); + warning (_("%d\t%ps: %s"), line, + styled_string (file_name_style.style (), filename), + safe_strerror (errcode)); } else { diff --git a/gdb/symfile.c b/gdb/symfile.c index 145621f3c67..10074e281bd 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -2483,9 +2483,8 @@ reread_symbols (int from_tty) int res = stat (filename, &new_statbuf); if (res != 0) { - /* FIXME, should use print_sys_errmsg but it's not filtered. */ - gdb_printf (_("`%ps' has disappeared; keeping its symbols.\n"), - styled_string (file_name_style.style (), filename)); + warning (_("`%ps' has disappeared; keeping its symbols."), + styled_string (file_name_style.style (), filename)); continue; } new_modtime = new_statbuf.st_mtime; diff --git a/gdb/utils.c b/gdb/utils.c index 2f545337cd4..a191d26a007 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -619,14 +619,13 @@ perror_warning_with_name (const char *string) warning (_("%s"), combined.c_str ()); } -/* Print the system error message for ERRCODE, and also mention STRING - as the file name for which the error was encountered. */ +/* See utils.h. */ void -print_sys_errmsg (const char *string, int errcode) +warning_filename_and_errno (const char *filename, int saved_errno) { - const char *err = safe_strerror (errcode); - gdb_printf (gdb_stderr, "%s: %s.\n", string, err); + warning (_("%ps: %s"), styled_string (file_name_style.style (), filename), + safe_strerror (saved_errno)); } /* Control C eventually causes this to be called, at a convenient time. */ diff --git a/gdb/utils.h b/gdb/utils.h index c5364fa4b35..f646b300530 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -275,7 +275,13 @@ extern void fprintf_symbol (struct ui_file *, const char *, extern void perror_warning_with_name (const char *string); -extern void print_sys_errmsg (const char *, int); +/* Issue a warning formatted as ': ', where + is FILENAME with filename styling applied. As such, don't + pass anything more than a filename in this string. The + is a string returned from calling safe_strerror(SAVED_ERRNO). */ + +extern void warning_filename_and_errno (const char *filename, + int saved_errno); /* Warnings and error messages. */ diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 5a897dbfe76..7a139c8d36f 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -2625,7 +2625,7 @@ windows_nat_target::create_inferior (const char *exec_file, tty = open (inferior_tty.c_str (), O_RDWR | O_NOCTTY); if (tty < 0) { - print_sys_errmsg (inferior_tty.c_str (), errno); + warning_filename_and_errno (inferior_tty.c_str (), errno); ostdin = ostdout = ostderr = -1; } else