From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14167 invoked by alias); 16 Apr 2012 02:07:06 -0000 Received: (qmail 13775 invoked by uid 22791); 16 Apr 2012 02:07:04 -0000 X-SWARE-Spam-Status: No, hits=0.1 required=5.0 tests=AWL,BAYES_00,KHOP_DYNAMIC2,RDNS_DYNAMIC,TW_EG X-Spam-Check-By: sourceware.org Received: from 173-13-178-50-sfba.hfc.comcastbusiness.net (HELO sebabeach.org) (173.13.178.50) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 16 Apr 2012 02:06:50 +0000 Received: by sebabeach.org (Postfix, from userid 500) id 554C311F7BD; Sun, 15 Apr 2012 19:06:49 -0700 (PDT) From: Doug Evans To: gdb-patches@sourceware.org Subject: [RFA] Ensure result of make_cleanup is never NULL. Date: Mon, 16 Apr 2012 05:01:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2012-04/txt/msg00398.txt.bz2 Hi. This is a followup patch to: http://sourceware.org/ml/gdb-patches/2012-04/msg00395.html [tweaked as indicated here: http://sourceware.org/ml/gdb-patches/2012-04/msg00396.html] It does two main things: - moves definition of struct cleanup into cleanups.c - ensures result of make_cleanup is never NULL There's more than one way to ensure the result of make_cleanup is never NULL. This way seems simple enough. Regression tested on amd64-linux. Ok to check in? 2012-04-15 Doug Evans * cleanups.h (struct cleanup): Move to cleanups.c. (make_cleanup_dtor_ftype): New typedef. (make_cleanup_dtor): Use it. (ALL_CLEANUPS): Replace with ... (all_cleanups): ... this. Declare. All uses updated. * utils.c (CLEANUP_FENCEPOST): Define. (cleanup_chain, final_cleanup_chain): Initialize to CLEANUP_FENCEPOST. (make_my_cleanup2): Assert result is non-NULL. (all_cleanups): New function. (save_my_cleanups): Initialize new chain to CLEANUP_FENCEPOST instead of NULL. --- cleanups.h= 2012-04-15 18:45:20.138903764 -0700 +++ cleanups.h 2012-04-15 18:37:45.916900087 -0700 @@ -19,29 +19,6 @@ #ifndef CLEANUPS_H #define CLEANUPS_H -/* The cleanup list records things that have to be undone - if an error happens (descriptors to be closed, memory to be freed, etc.) - Each link in the chain records a function to call and an - argument to give it. - - Use make_cleanup to add an element to the cleanup chain. - Use do_cleanups to do all cleanup actions back to a given - point in the chain. Use discard_cleanups to remove cleanups - from the chain back to a given point, not doing them. - - If the argument is pointer to allocated memory, then you need - to additionally set the 'free_arg' member to a function that will - free that memory. This function will be called both when the cleanup - is executed and when it's discarded. */ - -struct cleanup - { - struct cleanup *next; - void (*function) (void *); - void (*free_arg) (void *); - void *arg; - }; - /* NOTE: cagney/2000-03-04: This typedef is strictly for the make_cleanup function declarations below. Do not use this typedef as a cast when passing functions into the make_cleanup() code. @@ -49,21 +26,25 @@ struct cleanup Calling a f(char*) function with f(void*) is non-portable. */ typedef void (make_cleanup_ftype) (void *); +/* Function type for the dtor in make_cleanup_dtor. */ +typedef void (make_cleanup_dtor_ftype) (void *); + /* WARNING: The result of the "make cleanup" routines is not the intuitive choice of being a handle on the just-created cleanup. Instead it is an opaque handle of the cleanup mechanism and represents all cleanups created - from that point onwards. */ + from that point onwards. + The result is guaranteed to be non-NULL though. */ extern struct cleanup *make_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_cleanup_dtor (make_cleanup_ftype *, void *, - void (*dtor) (void *)); + make_cleanup_dtor_ftype *); extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); /* A special value to pass to do_cleanups and do_final_cleanups to tell them to do all cleanups. */ -#define ALL_CLEANUPS ((struct cleanup *)0) +extern struct cleanup *all_cleanups (void); extern void do_cleanups (struct cleanup *); extern void do_final_cleanups (struct cleanup *); --- cleanups.c= 2012-04-15 11:20:33.999687721 -0700 +++ cleanups.c 2012-04-15 18:46:51.397904493 -0700 @@ -18,15 +18,43 @@ along with this program. If not, see . */ #include "defs.h" +#include "gdb_assert.h" + +/* The cleanup list records things that have to be undone + if an error happens (descriptors to be closed, memory to be freed, etc.) + Each link in the chain records a function to call and an + argument to give it. + + Use make_cleanup to add an element to the cleanup chain. + Use do_cleanups to do all cleanup actions back to a given + point in the chain. Use discard_cleanups to remove cleanups + from the chain back to a given point, not doing them. + + If the argument is pointer to allocated memory, then you need + to additionally set the 'free_arg' member to a function that will + free that memory. This function will be called both when the cleanup + is executed and when it's discarded. */ + +struct cleanup + { + struct cleanup *next; + void (*function) (void *); + void (*free_arg) (void *); + void *arg; + }; + +/* A fencepost used to mark the end of a cleanup chain. + The value is chosen to be non-NULL so that make_cleanup never returns NULL, + and cause a segv if dereferenced. */ +#define CLEANUP_FENCEPOST ((struct cleanup *) 1) /* Chain of cleanup actions established with make_cleanup, to be executed if an error happens. */ +static struct cleanup *cleanup_chain = CLEANUP_FENCEPOST; -/* Cleaned up after a failed command. */ -static struct cleanup *cleanup_chain; - -/* Cleaned up when gdb exits. */ -static struct cleanup *final_cleanup_chain; +/* Chain of cleanup actions established with make_final_cleanup, + to be executed when gdb exits. */ +static struct cleanup *final_cleanup_chain = CLEANUP_FENCEPOST; static struct cleanup * make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function, @@ -42,6 +70,7 @@ make_my_cleanup2 (struct cleanup **pmy_c new->arg = arg; *pmy_chain = new; + gdb_assert (old_chain != NULL); return old_chain; } @@ -98,6 +127,15 @@ do_my_cleanups (struct cleanup **pmy_cha } } +/* Return a value that can be passed to do_cleanups, do_final_cleanups to + indicate perform all cleanups. */ + +struct cleanup * +all_cleanups (void) +{ + return CLEANUP_FENCEPOST; +} + /* Discard cleanups and do the actions they describe until we get back to the point OLD_CHAIN in the cleanup_chain. */ @@ -154,7 +192,7 @@ save_my_cleanups (struct cleanup **pmy_c { struct cleanup *old_chain = *pmy_chain; - *pmy_chain = 0; + *pmy_chain = CLEANUP_FENCEPOST; return old_chain; } Index: exceptions.c =================================================================== RCS file: /cvs/src/src/gdb/exceptions.c,v retrieving revision 1.50 diff -u -p -r1.50 exceptions.c --- exceptions.c 4 Jan 2012 08:17:01 -0000 1.50 +++ exceptions.c 16 Apr 2012 01:54:30 -0000 @@ -224,7 +224,7 @@ throw_exception (struct gdb_exception ex quit_flag = 0; immediate_quit = 0; - do_cleanups (ALL_CLEANUPS); + do_cleanups (all_cleanups ()); /* Jump to the containing catch_errors() call, communicating REASON to that call via setjmp's return value. Note that REASON can't Index: main.c =================================================================== RCS file: /cvs/src/src/gdb/main.c,v retrieving revision 1.104 diff -u -p -r1.104 main.c --- main.c 19 Mar 2012 18:19:24 -0000 1.104 +++ main.c 16 Apr 2012 01:54:30 -0000 @@ -230,7 +230,7 @@ captured_command_loop (void *data) are not that well behaved. do_cleanups should either be replaced with a do_cleanups call (to cover the problem) or an assertion check to detect bad FUNCs code. */ - do_cleanups (ALL_CLEANUPS); + do_cleanups (all_cleanups ()); /* If the command_loop returned, normally (rather than threw an error) we try to quit. If the quit is aborted, catch_errors() which called this catch the signal and restart the command Index: top.c =================================================================== RCS file: /cvs/src/src/gdb/top.c,v retrieving revision 1.214 diff -u -p -r1.214 top.c --- top.c 1 Mar 2012 19:30:20 -0000 1.214 +++ top.c 16 Apr 2012 01:54:30 -0000 @@ -1297,8 +1297,9 @@ quit_target (void *arg) if (write_history_p && history_filename) write_history (history_filename); - do_final_cleanups (ALL_CLEANUPS); /* Do any final cleanups before - exiting. */ + /* Do any final cleanups before exiting. */ + do_final_cleanups (all_cleanups ()); + return 0; }