[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux

[Ravitheja Addepally via Phabricator via lldb-commits]

ravitheja added inline comments.


================
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:158
+    LLDB_LOG(log, "ProcessorTrace failed to open Config file");
+    error.SetError(FileNotFound, eErrorTypePOSIX);
+    return error;
----------------
labath wrote:
> ravitheja wrote:
> > labath wrote:
> > > eErrorTypePOSIX is used for errno error values. Please don't try to pass your invented error codes as these.
> > Yes I did not want to use eErrorTypePOSIX but when transitioning from Status to llvm::Error, the m_code is only retained for eErrorTypePOSIX else its not retained.
> That's a good point. When I wrote the conversion function, there was no use case for this --  I think you're the first person who actually want's to use the error codes in some meaningful way.
> 
> What is your further plan for these error codes? Judging by the state of the D33035 you won't be able to use them to display the error messages to the user?
> 
> If you still want to preserve the error codes, we can definitely make this happen. Actually, llvm::Error makes this even easier, as it allows you to define new error categories in a distributed way. Frankly, I think the your use of the "generic" error category with custom error codes is a bit of a hack. I think the intended usage of the Status class was to define your own ErrorType enum value and tag your errors with that (but that does not scale well to many different sources of error codes).
My plan is to perhaps implement a way to pass error strings along with the error packets, so that the tool in D33035 can directly use those strings. I guess then I can just use the eErrorTypeGeneric . 

So keeping that in mind I can just remove the error codes and replace them everywhere with the strings ? Although the tool will not work unless the error strings are transported .



https://reviews.llvm.org/D33674