[rfa] symfile.c - fix apparently uninitialized my_cleanups
Fernando Nasser
fnasser@cygnus.com
Mon Nov 27 08:25:00 GMT 2000
Michael Snyder wrote:
>
> Andrew Cagney wrote:
> >
> > Hello,
> >
> > I think the attatched is correct. ``my_cleanups'' was always
> > initialized but in a somewhat obscure way. It probably illustrates a
> > better way of doing cleanups for a function - unconditionally setup the
> > cleanup at the very start.
>
> Andrew,
>
> There's nothing wrong with your change, but since there are
> probably hundreds of other functions that do things in the
> same way this one did, do you want to say any more about
> why it's bad? Maybe motivate a few people to follow your
> example?
>
I believe that, with the initialization being inside an "if", the
compiler will issue a warning "my_cleanups may be used uninitialized...".
It is always good to get silly warnings out of the way so they don't clobber
the ones that really indicate something fishy.
Also, as a question of style, what Andrew is proposing is safer. Although
it is not necessary everywhere, it is good when cleanups are inside conditionals. If someone does
forget a path of execution that does not set
"my_cleanups" (or equivalent) and do call do_cleanups(my_cleanups) we may
end up cleaning up too much stuff.
Fernando
> Michael
>
> >
> > Ok?
> > Andrew
> >
> > ---------------------------------------------------------------
> > Mon Nov 27 18:19:46 2000 Andrew Cagney <cagney@b1.cygnus.com>
> >
> > * symfile.c (add_symbol_file_command): Always initialize
> > my_cleanup using a NULL cleanup.
> >
> > Index: symfile.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/symfile.c,v
> > retrieving revision 1.20
> > diff -p -r1.20 symfile.c
> > *** symfile.c 2000/10/27 15:02:42 1.20
> > --- symfile.c 2000/11/27 07:22:03
> > *************** add_symbol_file_command (char *args, int
> > *** 1416,1422 ****
> > } sect_opts[SECT_OFF_MAX];
> >
> > struct section_addr_info section_addrs;
> > ! struct cleanup *my_cleanups;
> >
> > dont_repeat ();
> >
> > --- 1417,1423 ----
> > } sect_opts[SECT_OFF_MAX];
> >
> > struct section_addr_info section_addrs;
> > ! struct cleanup *my_cleanups = make_cleanup (null_cleanup, NULL);
> >
> > dont_repeat ();
> >
> > *************** add_symbol_file_command (char *args, int
> > *** 1452,1458 ****
> > {
> > /* The first argument is the file name. */
> > filename = tilde_expand (arg);
> > ! my_cleanups = make_cleanup (free, filename);
> > }
> > else
> > if (argcnt == 1)
> > --- 1453,1459 ----
> > {
> > /* The first argument is the file name. */
> > filename = tilde_expand (arg);
> > ! make_cleanup (free, filename);
> > }
> > else
> > if (argcnt == 1)
--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9
More information about the Gdb-patches
mailing list