From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27505 invoked by alias); 17 May 2010 05:36:59 -0000 Received: (qmail 27492 invoked by uid 22791); 17 May 2010 05:36:57 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 17 May 2010 05:36:54 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 69DFE2BAB40; Mon, 17 May 2010 01:36:52 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id CvGkRXhROuI8; Mon, 17 May 2010 01:36:52 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id AD1F52BAB35; Mon, 17 May 2010 01:36:51 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 02675F58F9; Mon, 17 May 2010 07:36:47 +0200 (CEST) Date: Mon, 17 May 2010 06:37:00 -0000 From: Joel Brobecker To: Hui Zhu Cc: gdb-patches ml Subject: Re: [OB] targe.c: extern reset_schedlock Message-ID: <20100517053647.GB2805@adacore.com> References: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="WIyZ46R2i8wDzkSu" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) 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-05/txt/msg00335.txt.bz2 --WIyZ46R2i8wDzkSu Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 938 Hui, > target_mourn_inferior (void) > { > struct target_ops *t; > + extern void reset_schedlock (); This sort of local declaration should *not* be used except under special circumstances. Declarations inside a .c, a fortiori inside a subprogram, are a maintenance hazard, because the compiler can not check for you that this declaration matches the actual definition. In other words, if someone changes the function profile, and forgets to update this call site, then you'll have a program that still compiles without warning, but whose execution becomes undefined. Another tiny detail is the fact that functions without arguments should have a "void" parameter, as opposed to nothing. In other words, replace: extern void reset_schedlock (); by: extern void reset_schedlock (void); Attached is a patch I just checked in to correct both issues. I also improved the original patch in terms of the comments. -- Joel --WIyZ46R2i8wDzkSu Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="reset_schedlock.diff" Content-length: 2641 commit 59b929a962c497fdc4002817f65917966972a818 Author: Joel Brobecker Date: Sun May 16 22:25:53 2010 -0700 Add reset_schedlock declaration in target.h. This patches improves a couple of previous patches: - one that introduces reset_schedlock, but failed to add a declarationl; - one that was checked in to avoid a compilation failure due to that missing declaration. It also improves the declaration itself to better conform to our coding practices. Same for the comments. 2010-05-17 Joel Brobecker * target.h (reset_schedlock): Add declaration. * infrun.c (reset_schedlock): Add missing void in function profile. * target.c (target_mourn_inferior): Delete local declaration of reset_schedlock. Style-fix in comment. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index cb272cd..5d35d00 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2010-05-17 Joel Brobecker + + * target.h (reset_schedlock): Add declaration. + * infrun.c (reset_schedlock): Add missing void in function profile. + * target.c (target_mourn_inferior): Delete local declaration of + reset_schedlock. Style-fix in comment. + 2010-05-17 Hui Zhu * target.c (target_mourn_inferior): Extern reset_schedlock. diff --git a/gdb/infrun.c b/gdb/infrun.c index 96da4cb..025ba0a 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1420,10 +1420,11 @@ set_schedlock_func (char *args, int from_tty, struct cmd_list_element *c) } } -/* reset_schedlock -- public */ +/* If SCHEDULER_MODE is on, then set it back to off. Warn the user + about the change. */ void -reset_schedlock () +reset_schedlock (void) { if (scheduler_mode == schedlock_on) { diff --git a/gdb/target.c b/gdb/target.c index cee3582..ebcf51c 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2244,9 +2244,8 @@ void target_mourn_inferior (void) { struct target_ops *t; - extern void reset_schedlock (); - /* Clear schedlock in infrun.c */ + /* Clear schedlock in infrun.c. */ reset_schedlock (); for (t = current_target.beneath; t != NULL; t = t->beneath) diff --git a/gdb/target.h b/gdb/target.h index d4bd007..cd2e82b 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -873,6 +873,8 @@ int target_write_memory_blocks (VEC(memory_write_request_s) *requests, /* From infrun.c. */ +extern void reset_schedlock (void); + extern int inferior_has_forked (ptid_t pid, ptid_t *child_pid); extern int inferior_has_vforked (ptid_t pid, ptid_t *child_pid); --WIyZ46R2i8wDzkSu--