Re: [RFA] Ensure result of make_cleanup is never NULL.

[Doug Evans <dje@google.com>]

On Tue, Apr 17, 2012 at 4:00 PM, Doug Evans <dje@google.com> wrote:
> On Mon, Apr 16, 2012 at 8:21 AM, Doug Evans <dje@google.com> wrote:
>>
>> On Apr 16, 2012 7:58 AM, "Pedro Alves" <palves@redhat.com> wrote:
>>>
>>> On 04/16/2012 03:40 PM, Joel Brobecker wrote:
>>>
>>> >> +/* 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)
>>> >
>>> > Just a tiny idea, not really important, but JIC: Could we use an enum
>>> > intead of a define so that GDB prints "CLEANUP_FENCEPOST" rather than
>>> > a numeric value when we print a cleanup pointer that's the fencepost?
>>>
>>>
>>> No, because what you'll be printing will have type struct cleanup pointer,
>>> not
>>> whatever enum it was cast from.
>>>
>>> I see at least two ways to get something like that:
>>>
>>> - a gdb specific pretty printer for cleanups.
>>>
>>> - Make the sentinel a real object:
>>>
>>>   static struct cleanup sentinel_cleanup;
>>>   #define CLEANUP_FENCEPOST &sentinel_cleanup
>>>
>>>  And get Tromey's "set print symbol" patch in, which IIRC/IIUC, the
>>>  latest version makes GDB print the symbol name corresponding
>>>  to addresses by default.  Then gdb would print something like:
>>>
>>>   (gdb) p old_chain
>>>   $1 = 0xfoobar <sentinel_cleanup>
>>>
>>> --
>>> Pedro Alves
>>
>> That was my other plan.  I can do that instead.
>
> How about this?
>
>
> 2012-04-17  Doug Evans  <dje@google.com>
>
>        * 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.
>        * cleanups.c (cleanup_sentinel): New static global.
>        (CLEANUP_SENTINEL): Define.
>        (cleanup_chain, final_cleanup_chain): Initialize to CLEANUP_SENTINEL.
>        (make_my_cleanup2): Assert result is non-NULL.
>        (all_cleanups): New function.
>        (save_my_cleanups): Initialize new chain to CLEANUP_SENTINEL instead
>        of NULL.

btw, I thought about making cleanup_sentinel const here, for a bit of
extra robustness.