This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] arm reversible : <phase_2_complete>
- From: Tom Tromey <tromey at redhat dot com>
- To: oza Pawandeep <oza dot pawandeep at gmail dot com>
- Cc: Petr HluzÃn <petr dot hluzin at gmail dot com>, paawan oza <paawan1982 at yahoo dot com>, "gdb-patches\ at sourceware dot org" <gdb-patches at sourceware dot org>, chandra krishnappa <chandra_roadking at yahoo dot com>
- Date: Fri, 02 Dec 2011 11:36:36 -0700
- Subject: Re: [PATCH] arm reversible : <phase_2_complete>
- References: <998639.46560.qm@web112516.mail.gq1.yahoo.com> <m3fwnusq4e.fsf@fleche.redhat.com> <321260.58442.qm@web112504.mail.gq1.yahoo.com> <m3hb6rfc5j.fsf@fleche.redhat.com> <1316327455.23344.YahooMailNeo@web112509.mail.gq1.yahoo.com> <1316404058.27177.YahooMailNeo@web112502.mail.gq1.yahoo.com> <m3pqhzcowx.fsf@fleche.redhat.com> <1318650316.91503.YahooMailNeo@web112508.mail.gq1.yahoo.com> <CAC=yr6D+d-Q4Yzfm6AoWSYgb4+Bopubg4wFDX9QDSUUFFcFtWg@mail.gmail.com> <m3lirxrtrq.fsf@fleche.redhat.com> <CAC=yr6BAsuR-_Z6dbqYBXkKESCX7CkKOuUAYYtRFcT6==Mo5Gw@mail.gmail.com> <m37h3cndru.fsf@fleche.redhat.com> <CAK1A=4wzgTGWGsckjnMSC24yfq97YC6MYnMyp0VK2Dc+zpjx6w@mail.gmail.com> <CAK1A=4yYqH9mHuDVmMAD8d9y5X176240pxDr4Ac3bVs8p5jQhw@mail.gmail.com> <m3vcqieb48.fsf@fleche.redhat.com> <CAK1A=4wz=2uU71yFz39EdOMvrLEyR+dX9TEogJ6vRR2Pu4pDNw@mail.gmail.com> <CAK1A=4zrPC2WnP8_F+pF9djgxDsZJMu6hbVMVVZXvC-VoqOHjQ@mail.gmail.com>
>>>>> "oza" == oza Pawandeep <oza.pawandeep@gmail.com> writes:
oza> please find the updated patch; I have tried my level best to take care
oza> the most to get formatting right.
oza> And also, all extra parentheses removed.
Thanks.
oza> please have a look. (hope mailer does not do anything)
Your mailer is still screwed up.
You didn't write a ChangeLog entry or a NEWS entry.
Please do this. I have asked at least once now. These are just
baseline things for getting patches into gdb; if you don't respond to
these requests it makes me tend to prioritize other patches over yours.
FYI.
oza> + /* if this insn has fallen into extension space
oza> + then we need not decode it anymore. */
oza> + if (ret != -1 && !INSN_RECORDED(arm_record))
oza> + {
oza> + arm_handle_insn[insn_id] (arm_record);
Still not checking the return value.
oza> +/* cleans up local record registers and memory allocations. */
Should start with a capital letter.
Blank line between comment and function.
oza> +void deallocate_reg_mem(insn_decode_record *record)
Newline after void.
Space before open paren.
oza> +{
oza> + if (record->arm_regs)
oza> + xfree (record->arm_regs);
oza> + if (record->arm_mems)
oza> + xfree (record->arm_mems);
oza> +}
You can call xfree on a NULL pointer. Just remove those ifs.
oza> + for (no_of_rec = 0; no_of_rec < arm_record.mem_rec_count;
oza> no_of_rec++)
oza> + {
Brace indentation looks wrong.
There are still other minor formatting issues. I saw 21 places where
there is a comma at the start of a line, please fix all of these.
I think there were some other problems, too, but at this point I have
stopped caring.
Tom