From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4028 invoked by alias); 3 Sep 2010 13:04:35 -0000 Received: (qmail 3781 invoked by uid 22791); 3 Sep 2010 13:04:34 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,TW_TD,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 03 Sep 2010 13:04:29 +0000 Received: (qmail 27237 invoked from network); 3 Sep 2010 13:04:27 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 3 Sep 2010 13:04:27 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [patch] Fix nesting of ui_out_redirect Date: Fri, 03 Sep 2010 15:36:00 -0000 User-Agent: KMail/1.13.2 (Linux/2.6.33-29-realtime; KDE/4.4.2; x86_64; ; ) Cc: Jan Kratochvil , pebolle@tiscali.nl References: <20100903112139.GA20855@host1.dyn.jankratochvil.net> In-Reply-To: <20100903112139.GA20855@host1.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009031404.23364.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-09/txt/msg00124.txt.bz2 On Friday 03 September 2010 12:21:39, Jan Kratochvil wrote: > Hi, > > ui_out_redirect assumed only double level of redirections so far. > > GDB started to use them nested, though. The testcase crashes FSF GDB HEAD. > > Made there also some small related safety fixups of the related calls. > > SUPPRESS_UI_FILEP_DECL is very ugly there but I am not aware how to easily use > vec.h otherwise and vec.h was being pushed as the GDB data type in such cases. Just putting the DEF_VEC_P (ui_filep); in the cli-out.h header is the norm. It looks easy to tweak vec.h to get rid of the typedef, and so be able to forward declare VECs. E.g.,: --- gdb/cli-out.c | 6 ++---- gdb/cli-out.h | 4 +--- gdb/vec.h | 11 ++++++++--- 3 files changed, 11 insertions(+), 10 deletions(-) Index: src/gdb/cli-out.c =================================================================== --- src.orig/gdb/cli-out.c 2010-09-03 12:43:35.000000000 +0100 +++ src/gdb/cli-out.c 2010-09-03 13:42:01.000000000 +0100 @@ -23,15 +23,13 @@ #include "defs.h" #include "vec.h" -#define SUPPRESS_UI_FILEP_DECL -typedef struct ui_file *ui_filep; -DEF_VEC_P (ui_filep); - #include "ui-out.h" #include "cli-out.h" #include "gdb_string.h" #include "gdb_assert.h" +DEF_VEC_P (ui_filep); + typedef struct cli_ui_out_data cli_out_data; Index: src/gdb/cli-out.h =================================================================== --- src.orig/gdb/cli-out.h 2010-09-03 12:43:35.000000000 +0100 +++ src/gdb/cli-out.h 2010-09-03 13:41:48.000000000 +0100 @@ -25,10 +25,8 @@ #include "vec.h" /* Used for cli_ui_out_data->streams. */ -#ifndef SUPPRESS_UI_FILEP_DECL typedef struct ui_file *ui_filep; -VEC_T (ui_filep); -#endif +DEC_VEC (ui_filep); /* These are exported so that they can be extended by other `ui_out' implementations, like TUI's. */ Index: src/gdb/vec.h =================================================================== --- src.orig/gdb/vec.h 2010-06-16 12:36:45.000000000 +0100 +++ src/gdb/vec.h 2010-09-03 13:45:33.000000000 +0100 @@ -392,16 +392,21 @@ extern void *vec_o_reserve (void *, int, #define vec_assert(expr, op) \ ((void)((expr) ? 0 : (gdb_assert_fail (op, file_, line_, ASSERT_FUNCTION), 0))) -#define VEC(T) VEC_##T +#define VEC_TAG(T) VEC_##T #define VEC_OP(T,OP) VEC_##T##_##OP +#define DEC_VEC(T) \ + struct VEC_TAG(T) + #define VEC_T(T) \ -typedef struct VEC(T) \ +struct VEC_TAG(T) \ { \ unsigned num; \ unsigned alloc; \ T vec[1]; \ -} VEC(T) +} + +#define VEC(T) struct VEC_TAG(T) /* Vector of integer-like object. */ #define DEF_VEC_I(T) \ but I'm really not sure it's worth it to have. Each module that wants to use the VEC still needs to "DEF_VEC_P (ui_filep)" (or similar), given that that defines the bunch of static inline functions that actually manipulate the VEC. We'd probably want something like this in the headers: DEC_VEC (ui_filep); #define DEF_VEC_ui_filep \ DEF_VEC_P (ui_filep) and then in the .c files that actually use the VEC, write DEF_VEC_ui_filep; somewhere at the top. (replace ui_filep with your favorite type name, and _P with _I or _O appropriately, of course.) > --- a/gdb/cli/cli-logging.c > +++ b/gdb/cli/cli-logging.c > @@ -118,9 +118,12 @@ set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c) > gdb_stdtarg = output; > logging_no_redirect_file = new_logging_no_redirect_file; > > - /* It should not happen, the redirection has been already setup. */ > - if (ui_out_redirect (uiout, output) < 0) > - warning (_("Current output protocol does not support redirection")); > + if (ui_out_redirect (uiout, NULL) < 0 > + || ui_out_redirect (uiout, output) < 0) > + { > + /* It should not happen, the redirection has been already setup. */ > + warning (_("Current output protocol does not support redirection")); > + } I'm feeling dense, and this change isn't looking obvious to me. Can you explain it? > > if (logging_redirect != 0) > do_cleanups (cleanups); > @@ -212,7 +215,7 @@ handle_redirections (int from_tty) > gdb_stdlog = output; > gdb_stdtarg = output; > > - if (ui_out_redirect (uiout, gdb_stdout) < 0) > + if (ui_out_redirect (uiout, output) < 0) > warning (_("Current output protocol does not support redirection")); > } > Otherwise, it looks good to me. Pedro Alves