From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <lsix@lancelotsix.com>
Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111])
 by sourceware.org (Postfix) with ESMTPS id 45F7C3858D39
 for <gdb-patches@sourceware.org>; Thu, 13 Jan 2022 08:51:04 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 45F7C3858D39
Received: from Plymouth (unknown [IPv6:2a02:390:9086:0:f1a2:ba70:4135:5b49])
 by lndn.lancelotsix.com (Postfix) with ESMTPSA id 9B71D80D5C;
 Thu, 13 Jan 2022 08:51:02 +0000 (UTC)
Date: Thu, 13 Jan 2022 08:50:59 +0000
From: Lancelot SIX <lsix@lancelotsix.com>
To: Tom Tromey <tromey@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2 3/3] Add a way to temporarily set a gdb parameter from
 Python
Message-ID: <20220113085059.z3rm44jun2sconai@Plymouth>
References: <20220110215144.1714909-1-tromey@adacore.com>
 <20220110215144.1714909-4-tromey@adacore.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220110215144.1714909-4-tromey@adacore.com>
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11
 (lndn.lancelotsix.com [0.0.0.0]); Thu, 13 Jan 2022 08:51:02 +0000 (UTC)
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, DKIM_SIGNED,
 DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS,
 TXREP autolearn=ham autolearn_force=no version=3.4.4
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on
 server2.sourceware.org
X-BeenThere: gdb-patches@sourceware.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org>
List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>,
 <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe>
List-Archive: <https://sourceware.org/pipermail/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help>
List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>,
 <mailto:gdb-patches-request@sourceware.org?subject=subscribe>
X-List-Received-Date: Thu, 13 Jan 2022 08:51:07 -0000

> +
> +
> +@contextmanager
> +def with_parameter(name, value):
> +    """Temporarily set the GDB parameter NAME to VALUE.
> +    Note that this is a context manager."""
> +    old_value = parameter(name)
> +    set_parameter(name, value)
> +    # Nothing that useful to return.
> +    yield None
> +    set_parameter(name, old_value)
> +
> +

Hi,

In python, context managers are usually used the way RAII resources are
in c++. which is to ensure some resource is reliably released (parameter
is restored in this case) at the end of the block.  Here, if an
exception is thrown within the block, the original value is not
restored.

I would suggest something like:


    @contextmanager
    def with_parameter(name, value):
        """Temporarily set the GDB parameter NAME to VALUE.
        Note that this is a context manager."""
        old_value = parameter(name)
        set_parameter(name, value)
        try:
            yield
        finally:
            set_parameter(name, old_value)


This way, the parameter is restored even in case of exception, which is
what I would expect from a python user point of view.

Best,
Lancelot.