[llvm-commits] Error-handling fixes for the x86 disassembler
Chris Lattner
clattner at apple.com
Thu Apr 1 20:13:59 PDT 2010
On Apr 1, 2010, at 6:48 PM, Sean Callanan wrote:
> We talked about this a little off-list, and decided to have the disassembler return an error code in all cases, but only print a debug message on NDEBUG builds. The idea was to use the DEBUG macro and debugs() stream.
>
> The attached patch implements these updates. Please let me know what you think.
The patch generally looks good to me, remind me again why X86DisassemblerDecoder.c has to be a C file though?
- translateInstruction(instr, internalInstr);
- return true;
+ if (translateInstruction(instr, internalInstr))
+ return false;
+ else
+ return true;
This can just be: return !translateInstruction(...
However, it looks like translateInstruction can just return a bool instead of 0/-1. If you do this, please use true for error to follow other llvm precedent, and allowing the ! to go away. Likewise for translateRM etc.
+ if (!(insn.eaBase != EA_BASE_sib && insn.eaBase != EA_BASE_sib64)) {
Please apply demorgan's law to simplify the condition.
+ if (!(stackPos < 8)) {
Please simplify.
+#ifndef NDEBUG
+#define debug(s) do { x86DisassemblerDebug(__FILE__, __LINE__, s); } while (0);
+#else
+#define debug(s) do { } while (0);
+#endif
The typical do/while(0) idiom doesn't include the trailing ; in the macro.
Otherwise, looks fine, please commit with changes,
-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100401/6c03589a/attachment.html>
More information about the llvm-commits
mailing list