This is the mail archive of the gdb-patches@sourceware.cygnus.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: make_cleanup_func elimination


> The main thing to note is that the reason I didn't rush back and fix the
> make_cleanup_func botch (ok so JasonM helped me :-) was because, as far
> as anyone knows (gcc people here), while the code is technically in
> violation of ISO-C there isn't a mainstream platform on which the hack
> will actually fail.

That is true of *MOST* of the things I fixed (which involve void*
versus another pointer - C++ being the only problem I know of for
that, which isn't relevant as such).  And others, like "int"
vs. pointer where the argument is never used, also will probably
happen to work (even if sizeof int != sizeof pointer).  I don't
remember for sure whether I found any which will fail on today's
mainstream 64 bit machines.  But my experience (for whatever it is
worth) is that it is less work to just fix such things than to explain
over and over (including to oneself :-)) why they are only partially
fixed.

IMHO, this kind of fix is low-risk and therefore I wouldn't worry as
much about delaying it past the release as I would have some years
ago.  Of course, that is a matter of opinion and you are right that
there are different ways to fix the problem.  If anyone is thinking of
jumping in, don't get scared, the patches I wrote only took a few
hours, so anyone can play.

> This is why I've been slowly going through and adding functions like:
> extern struct cleanup *make_cleanup_ui_file_delete (struct ui_file *);

Yeah, I saw those.  Although the type-checking that it provides is
good, the #1 thing which deterred me was that this provides no way to
call the make_my_cleanup family of functions.

> -static void free_variable PARAMS ((struct varobj * var));
> +static void free_variable (void *);
> 
> The original function was strongly typed.  By changing it to void* that
> type information is lost and the chance of incorrect use increased.

For me it was case by case.  If the function was being used *only* as
a cleanup, I'd figure that the type checking wasn't really doing much
(or likewise and more arguably if there was only one non-cleanup call
as with free_variable).  Similarly if the argument was unused.  In a
lot of other cases I introduced a cleanup_* function which takes the
void* and passes the struct varobj* or whatever it is.

> If you want to pursue this

No, not really, I'm largely losing interest in this kind of work
(which could be its own long rant, but not one that would necessarily
advance the cause of GDB hacking :-)), and posting the
make_cleanup_func patch to the list was part of cleaning out my
backlog of things I'd started and not finished.

Now, if you want an opinion, I don't think you should make the person
who takes this on next go to each and every maintainer whose code is
touched.  That makes it a daunting job and IMHO isn't needed for this
particular situation.  But it is your call rather than mine.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]