[llvm-commits] Error-handling fixes for the x86 disassembler

[Sean Callanan]

Eric, Daniel,

thanks for your comments.  I've updated the patch to reflect your concerns:

- I return 0 or -1 in all cases, and never just plain return.  (0 is sometimes the right thing to return, given the return type.)
- All fprintf(stderr)s are #ifdef'd out pending a proper error-reporting API, which I'll propose separately
- Spaces have been inserted between if/for/switch and ( where I missed them
- I went through and audited all function-like macros:
	The error macro is useful because it uses __FILE__ and __LINE__.
	The CONSUME_FUNC macro defines a static function.  It is essentially a template but in C I can't use a template.
	The GENERIC_FIXUP_FUNC macro defines a static function.  It is a template but does something (adding prefixes to the head of an enum name) that not even a template could do.
  In short, the function-like macros I have save considerable code and perform functions that regular functions could not.

I think we ought to discuss a more general error-retrieval API on llvmdev, but I don't want to backpack this patch, which needs to be resolved pretty quickly, onto that discussion.

Sean

-------------- next part --------------
A non-text attachment was scrubbed...
Name: x86-disassembler-assert.diff
Type: application/octet-stream
Size: 26999 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100331/6424b914/attachment.obj>
-------------- next part --------------


On Mar 31, 2010, at 4:24 PM, Eric Christopher wrote:

> 
> On Mar 31, 2010, at 3:42 PM, Sean Callanan wrote:
> 
>> The attached patch fixes error handling for the x86 disassembler, eliminating llvm_unreachable() and assert() in favor of using the legitimate error-handling mechanism.  The rationale is to allow clients of libEnhancedDisassembly to handle errors themselves, rather than forcing them to crash.
>> 
>> The difference is that now assert() and llvm_unreachable() will never be called (even before, this was extremely unlikely), and the proper error-handling mechanism is used instead.  Further diagnostic information is printed to stderr.
>> 
>> Please let me know what you think.
> 
> I can't tell with some of the diffs without looking closer but it appears that you have 3 different courses of action when you encounter an error:
> 
> a) print something, return -1
> b) print something, return 0
> c) print something, return
> 
> Is there any way to unify some of this? Perhaps even an enum with different types of errors for consumers?
> 
> -eric