[ESI][Runtime] MMIO service support #6034
Conversation
Service C++ interface, Python wrapping, and cosim support. Also adds the basic service framework.
| std::unique_ptr<Service> &cacheEntry = serviceCache[&svcType]; | ||
| if (cacheEntry != nullptr) | ||
| return cacheEntry.get(); | ||
| cacheEntry = std::unique_ptr<Service>(createService(svcType)); | ||
| return cacheEntry.get(); | ||
| } |
There was a problem hiding this comment.
use std::map::emplace instead.
There was a problem hiding this comment.
Getting a reference to the storage slot saves a little code (which I fucked up) and a search for the insertion.
| virtual std::string rawJsonManifest() const = 0; | ||
| protected: | ||
| using Service = services::Service; | ||
| virtual Service *createService(Service::Type service) = 0; |
There was a problem hiding this comment.
Please use smart pointers here or write a comment to be very explicit about the ownership of the returned pointer (i see a mix of unique_ptrs being placed into caches and raw-pointers being allocated and return to the former).
There was a problem hiding this comment.
This is a protected method because it is not intended to be called directly by clients. It is called by getServiceImpl, which immediately wraps it in a unique pointer and adds it to the cache. I structured it that way to avoid subclasses from having to reason about the way that the cache is implemented. I will add some documentation to that effect.
| Service *CosimAccelerator::createService(Service::Type svcType) { | ||
| if (svcType == typeid(MMIO)) | ||
| return new CosimMMIO(impl->lowLevel, impl->waitScope); | ||
| else if (svcType == typeid(SysInfo)) | ||
| return new MMIOSysInfo(getService<MMIO>()); | ||
| return nullptr; |
There was a problem hiding this comment.
To be frank, i don't think there is any merit in this kind of code returning a raw pointer. If you really want the raw pointer semantics, then there should be a Service* getService method that dispatches to a std::unique_ptr<Service> createService function in case the service was not found in the cache.
There was a problem hiding this comment.
That's what I do, but I wrap the pointer in another method.
Service C++ interface, Python wrapping, and cosim support for MMIO. Also adds the basic service framework.