From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Molenda To: Andrew Cagney Cc: gdb-patches@sources.redhat.com, Tom Tromey Subject: Re: [RFA] patch to add 'maint profile-gdb' command Date: Mon, 10 Sep 2001 11:52:00 -0000 Message-id: <20010910115244.A25119@shell17.ba.best.com> References: <20010910003022.A21681@shell17.ba.best.com> <3B9CE0C6.5060700@cygnus.com> X-SW-Source: 2001-09/msg00144.html On Mon, Sep 10, 2001 at 11:48:22AM -0400, Andrew Cagney wrote: > some brief, one handed, notes :-) Oh jeez, don't tell me you broke your arm again?! :-) > i think the command should always be present. looking at the patch, it > appears to have started out that way. I'd disagree. First, these functions are not very portable. Our very own RH 6.2 box that is sourceware.cygnus.com does not have moncontrol() or monstart(), and -pg doesn't seem to be usable at all. My RH 7.1 box at home works fine. MacOS X's FreeBSD works fine. Second, you definitely don't want to compile gdb with -pg by default - the compiler inserts a bunch of bookkeeping code and functions, and that'll be a real performance penalty. > hence, i wonder if ``-gdb'' is needed > in the command name? Perhaps ``(gdb) maint profile control [on|off]''? > This is really a cli maintainer question and outside my domain. Personally, I think it could just be "maint profile [on|off]", but that could be ambiguous - are you profiling gdb or its inferior? > are you sure no changes to maint.exp are needed? not needing them feels > wrong. The "help maint" test in maint.exp globs it up. You don't want to include a check for maint profile-gdb in here because you'd get a FAIL when gdb was not configured --enable-profiling. > suggest adding a ``post 5.1'' line to news - otherwise i'll get > confused. thanks for thinking of the news file. All credit goes to Tom, of course. > (Eli?) should the doco include a reference to moncontrol(3)? I don't see what you're thinking of. > suggest a comment against the moncontrol() call in main.c explaining the > problems. would having the call in main() be better - turn it off at > the earliest possible moment? I just re-read the man page, it doesn't matter where it's disabled. The gmon.out file is going to be overwritten each time gdb is run no matter what. If you're profiling something that takes real CPU, the start-up functions to get to control_main are background noise. > gdb is pure iso c, the params forward decl isn't needed. you may want > to look at http://sources.redhat.com/gdb/ari/ for a check list of things > recently zapped from gdb. The forward decl in maint.c? Tom was following the convention of the other functions in that file.. Jason