From patchwork Wed Feb 26 20:05:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 38331 Received: (qmail 101570 invoked by alias); 26 Feb 2020 20:06:32 -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 101376 invoked by uid 89); 26 Feb 2020 20:06:30 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=1999, controversial X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Feb 2020 20:06:28 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582747587; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=IZty+NlgzqQ4NoVX1juu+3Q/p1oH0oXjEBJgNgFCKMA=; b=YkAQVYnW5+BQU1F/W6r/nCB9XNwOFIwgvoTaIhMMu79oYx29EPWIAzQ5gBUesJyBPyq7BB 0xazAFA4aII7yDgQVTgJWbKE/Dw2fVLMoZJ42yLplODKcXHBnvcKB7BlFdS+ErIJpj/Kw6 GJzwXmyUUf27JcZ/CZrqdlTeeJxktJ4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-237-A_2bKbIwN6GhfbodyW7qUA-1; Wed, 26 Feb 2020 15:06:20 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 55839192296D; Wed, 26 Feb 2020 20:06:19 +0000 (UTC) Received: from psique.yyz.redhat.com (unused-10-15-17-54.yyz.redhat.com [10.15.17.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id AD8145DA76; Wed, 26 Feb 2020 20:06:18 +0000 (UTC) From: Sergio Durigan Junior To: GDB Patches Cc: Pedro Alves , Tom Tromey , Eli Zaretskii , Ruslan Kabatsayev , Sergio Durigan Junior Subject: [PATCH 2/6] Don't reset errno/bfd_error on 'throw_perror_with_name' Date: Wed, 26 Feb 2020 15:05:38 -0500 Message-Id: <20200226200542.746617-3-sergiodj@redhat.com> In-Reply-To: <20200226200542.746617-1-sergiodj@redhat.com> References: <20190926042155.31481-1-sergiodj@redhat.com> <20200226200542.746617-1-sergiodj@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-IsSubscribed: yes Since this hunk may be a bit controversial, I decided to split it into a separate patch. This is going to be needed by the ptrace-error feature; GDB will need to be able to access the value of errno even after a call to our 'perror'-like functions. The idea here is that we actually *want* errno to propagate between our customized 'perror' calls. We currently have this code on 'throw_perror_with_name': /* I understand setting these is a matter of taste. Still, some people may clear errno but not know about bfd_error. Doing this here is not unreasonable. */ bfd_set_error (bfd_error_no_error); errno = 0; git blame tells me that this piece of code is pretty old; the commit that "introduced" it is: commit c906108c21474dfb4ed285bcc0ac6fe02cd400cc Author: Stan Shebs Date: Fri Apr 16 01:35:26 1999 +0000 Initial creation of sourceware repository so yeah... If we go to the POSIX specification for 'perror', it doesn't really say anything about whether errno should be preserved or not. It does, however, say that 'perror's messages should be the same as those returned by 'strerror', and 'strerror' is not supposed to alter errno if the call is successful. Maybe when our wrapper was written it was OK to modify errno, I don't know. But I'd like to propose that we stick to POSIX in this case. Another small hunk is the one that saves/restores errno on gdbserver's 'perror_with_name', but this one is pretty trivial, I think. gdb/ChangeLog: yyyy-mm-dd Sergio Durigan Junior * utils.c (throw_perror_with_name): Don't reset errno/bfd_error. gdbserver/ChangeLog: yyyy-mm-dd Sergio Durigan Junior * utils.cc (perror_with_name): Save/restore errno. --- gdb/utils.c | 6 ------ gdbserver/utils.cc | 2 ++ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/gdb/utils.c b/gdb/utils.c index 0b470120a2..df8add1afa 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -595,12 +595,6 @@ throw_perror_with_name (enum errors errcode, const char *string) { std::string combined = perror_string (string); - /* I understand setting these is a matter of taste. Still, some people - may clear errno but not know about bfd_error. Doing this here is not - unreasonable. */ - bfd_set_error (bfd_error_no_error); - errno = 0; - throw_error (errcode, _("%s."), combined.c_str ()); } diff --git a/gdbserver/utils.cc b/gdbserver/utils.cc index d88f4ac5ca..50ebba43ca 100644 --- a/gdbserver/utils.cc +++ b/gdbserver/utils.cc @@ -46,6 +46,7 @@ perror_with_name (const char *string) { const char *err; char *combined; + int saved_errno = errno; err = safe_strerror (errno); if (err == NULL) @@ -56,6 +57,7 @@ perror_with_name (const char *string) strcat (combined, ": "); strcat (combined, err); + errno = saved_errno; error ("%s.", combined); }