This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 5/7] Adds support for thumb32 instructions recording
- From: Pedro Alves <palves at redhat dot com>
- To: Omair Javaid <omair dot javaid at linaro dot org>
- Cc: gdb-patches at sourceware dot org, patches at linaro dot org
- Date: Mon, 06 Jan 2014 19:52:25 +0000
- Subject: Re: [PATCH v2 5/7] Adds support for thumb32 instructions recording
- Authentication-results: sourceware.org; auth=none
- References: <y> <cover dot 1388709773 dot git dot omair dot javaid at linaro dot org> <aab666e0a2f5469950acc54c4e652ca9724c3afd dot 1388709773 dot git dot omair dot javaid at linaro dot org>
On 01/03/2014 07:15 PM, Omair Javaid wrote:
> + switch (op1)
> + {
> + /* Store byte instructions. */
> + case 4:
> + case 0:
> + record_buf_mem[0] = 1;
> + break;
"break;" should be aligned with the above statement,
not with "case". Several instances of this.
> + /* Swap first half of 32bit thumb instruction with second half. */
> + arm_record->arm_insn = (arm_record->arm_insn >> 16) |
> + (arm_record->arm_insn << 16);
'|' goes on the next line. But then you'd need to wrap the whole
thing with an extra set of parens. Write instead:
arm_record->arm_insn
= (arm_record->arm_insn >> 16) | (arm_record->arm_insn << 16);
> +
> + insn_id = thumb2_record_decode_insn_handler (arm_record);
> +
> + if (insn_id != ARM_RECORD_SUCCESS)
> + {
> + arm_record_unsupported_insn(arm_record);
Missing space before parens.
> + ret = -1;
> + }
> }
> else
Otherwise looks fine to me.
Thanks,
--
Pedro Alves