From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11861 invoked by alias); 18 Mar 2002 16:03:12 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 11731 invoked from network); 18 Mar 2002 16:03:07 -0000 Received: from unknown (HELO cygnus.com) (205.180.230.5) by sources.redhat.com with SMTP; 18 Mar 2002 16:03:07 -0000 Received: from redhat.com (rtl.cygnus.com [205.180.230.21]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id IAA12304; Mon, 18 Mar 2002 08:03:04 -0800 (PST) Message-ID: <3C960F4F.B356B226@redhat.com> Date: Mon, 18 Mar 2002 08:03:00 -0000 From: Fernando Nasser Organization: Red Hat Canada X-Mailer: Mozilla 4.78 [en] (X11; U; Linux 2.4.9-21 i686) X-Accept-Language: en MIME-Version: 1.0 To: Andrew Cagney CC: gdb-patches@sources.redhat.com Subject: Re: [rfc/cli:rfa] Don't copy func() into show from set .. References: <3C953AF0.7040504@cygnus.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2002-03/txt/msg00309.txt.bz2 Andrew, I agree with all that you say before but I have some additional thoughts. Should we ever allow for the creation of a set command without a show? If not, we should get rid of add_set_cmd() and add_show_from_set() and have a single add_setshow_cmd(). This would prevent anyone of creating a set without the corresponding show and hide the implementation details.  It could return the show pointer as an argument if so requested. One day we should have an ui independent way to register the variables and let the UIs create the appropriate commands themselves... Regards, Fernando Andrew Cagney wrote: > > Hello, > > The current add_show_from_set() uses memcpy() to clone the ``set'' > command into a ``show'' command. The function copies everything > including ``set''.func(), the command's callback. > > I think doing this is wrong. The ``show'' command should only pickup > directly relevant fields from ``set''. Any others should be set > separatly and explicitly. > > The first problem I see is with the behavour. The sequence: > c = add_set_cmd (...) > set_cmd_sfunc (c, ...) > add_show_from_set (c) > has very different behavour to: > c = add_set_cmd (...) > add_show_from_set (c) > add_cmd_sfunc (c, ...) > Only in the former case does the ``show'' command get ``set''.func(). I > think instead a user should be expected to explicitly set > ``show''.func() vis: > c = add_set_cmd (...) > s = add_show_from_set (s); > set_cmd_sfunc (s, ...) > set_cmd_sfunc (c, ...) > or (i.e. order no longer matters): > c = add_set_cmd (...) > set_cmd_sfunc (c, ...) > s = add_show_from_set (s); > set_cmd_sfunc (s, ...) > > The second problem I see is with the unintended consequences. Because > ``show'' has silently picked up ``set''.func(), commands like ``info > set'' call it. For the most part this is benign. Since the set > variable hasn't changed, the ``set''.func() just resets everything back > to what it was. Only occasionally has someone noticed this > ``re-setting'' and found it necessary to ignore it (kod.c, infun.c, > cris-tdep.c). > > The attatched patch modifies the behavour of add_show_from_set() so that > it only copies an explicit subset of the fields from the ``set'' command. > > With the patch applied, I've so far found no regressions! > > Thoughts? Ok? > > Andrew > > PS: This change dates back to before the dawn of time (well at least to > before Cygnus's CVS repository). > -- Fernando Nasser Red Hat Canada Ltd. E-Mail: fnasser@redhat.com 2323 Yonge Street, Suite #300 Toronto, Ontario M4P 2C9