Fix handling of prop-get requests

.github/workflows/codeql.yml

@@ -57,8 +57,7 @@ jobs:
       with:
         languages: ${{ matrix.language }}
         build-mode: ${{ matrix.build-mode }}
-        queries: |
-          security-and-quality
+        queries: security-extended
 
     - name: Run manual build steps
       if: matrix.build-mode == 'manual'

client/APIs/UrmAPIs.cpp

@@ -215,8 +215,11 @@ int8_t getProp(const char* prop, char* buffer, size_t bufferSize, const char* de
         ASSIGN_AND_INCR(ptr8, MOD_RESTUNE);
         ASSIGN_AND_INCR(ptr8, REQ_PROP_GET);
 
+        uint64_t* ptr64 = (uint64_t*)ptr8;
+        ASSIGN_AND_INCR(ptr64, bufferSize);
+
         const char* charIterator = prop;
-        char* charPointer = (char*) ptr8;
+        char* charPointer = (char*) ptr64;
 
         while(*charIterator != '\0') {
             ASSIGN_AND_INCR(charPointer, *charIterator);
@@ -225,9 +228,6 @@ int8_t getProp(const char* prop, char* buffer, size_t bufferSize, const char* de
 
         ASSIGN_AND_INCR(charPointer, '\0');
 
-        uint64_t* ptr64 = (uint64_t*)charPointer;
-        ASSIGN_AND_INCR(ptr64, bufferSize);
-
         if(conn == nullptr || RC_IS_NOTOK(conn->initiateConnection())) {
             LOGE("RESTUNE_CLIENT", CONN_INIT_FAIL);
             return -1;

configs/ResourcesConfig.yaml

@@ -24,18 +24,7 @@ ResourceConfigs:
     LowThreshold: 0
     Permissions: "third_party"
     Modes: ["display_on", "doze"]
-    Policy: "lower_is_better"
-    ApplyType: "cluster"
-
-  - ResType: "0x01"
-    ResID: "0x0002"
-    Name: "RES_PM_QOS_LATENCY"
-    Path: "/sys/devices/system/cpu/cpu%d/power/pm_qos_resume_latency_us"
-    Supported: true
-    LowThreshold: 0
-    Permissions: "third_party"
-    Modes: ["display_on", "doze"]
-    Policy: "lower_is_better"
+    Policy: "pass_through"
     ApplyType: "core"
 
   - ResType: "0x03"
@@ -326,7 +315,7 @@ ResourceConfigs:
     Supported: true
     Permissions: "third_party"
     Modes: ["display_on", "doze"]
-    Policy: higher_is_better
+    Policy: "higher_is_better"
     ApplyType: "global"
 
   - ResType: "0x0a"
@@ -336,7 +325,7 @@ ResourceConfigs:
     Supported: true
     Permissions: "third_party"
     Modes: ["display_on", "doze"]
-    Policy: higher_is_better
+    Policy: "higher_is_better"
     ApplyType: "global"
 
   - ResType: "0x0a"
@@ -347,7 +336,8 @@ ResourceConfigs:
     LowThreshold: 0
     Permissions: "third_party"
     Modes: ["display_on", "doze"]
-    Policy: higher_is_better
+    Policy: "higher_is_better"
+    ApplyType: "global"
 
   - ResType: "0x0b"
     ResID: "0x0000"

contextual-classifier/ContextualClassifierInit.cpp

@@ -94,4 +94,4 @@ static ErrCode terminate(void *arg = nullptr) {
     return RC_SUCCESS;
 }
 
-RESTUNE_REGISTER_MODULE(MOD_CLASSIFIER, init, terminate);
+URM_REGISTER_MODULE(MOD_CLASSIFIER, init, terminate);

modula/Components/Logger.cpp

@@ -68,7 +68,7 @@ void Logger::log(int32_t level,
         }
 
         case RedirectOptions::LOG_TOFILE: {
-            std::ofstream logFile("log.txt", std::ios::app);
+            std::ofstream logFile("/tmp/log.txt", std::ios::app);
             if(logFile.is_open()) {
                 logFile<<"["<<timestamp<<"] ["<<tag<<"] ["<<levelStr<<"] "<<funcName<<": "<<message<<std::endl;
                 logFile.close();
@@ -111,7 +111,7 @@ void Logger::log(int32_t level,
         }
 
         case RedirectOptions::LOG_TOFILE: {
-            std::ofstream logFile("log.txt", std::ios::app);
+            std::ofstream logFile("/tmp/log.txt", std::ios::app);
             if(logFile.is_open()) {
                 logFile<<"["<<timestamp<<"] ["<<tag<<"] ["<<levelStr<<"] "<<funcName<<": "<<message<<std::endl;
                 logFile.close();

modula/CoreModules/Include/ComponentRegistry.h

@@ -31,7 +31,8 @@ class ComponentRegistry {
     static ModuleInfo getModuleInfo(ModuleID moduleID);
 };
 
-#define RESTUNE_REGISTER_MODULE(identifier, registration, teardown) \
+#define URM_REGISTER_MODULE(identifier, registration, teardown) \
         static ComponentRegistry CONCAT(_module, identifier)(identifier, registration, teardown);
 
+
 #endif

resource-tuner/core/Include/PropertiesRegistry.h

@@ -39,11 +39,9 @@ class PropertiesRegistry {
      * @brief Get the Property value corresponding to the given key
      * @param propertyName Property Name or Key
      * @param result If the property exists, the value will be stored in this argument
-     * @return int8_t:\n
-     *            - 1: if a property with the given name was found\n
-     *            - 0: otherwise
+     * @return size_t: Number of bytes written
      */
-    int8_t queryProperty(const std::string& propertyName, std::string& result);
+    size_t queryProperty(const std::string& propertyName, std::string& result);
 
     /**
      * @brief Modify the value of the property with the given name

resource-tuner/core/Include/RestuneInternal.h

@@ -38,33 +38,12 @@ void submitResProvisionRequest(Request* request, int8_t isVerified);
  * @param prop Name of the Property to be fetched.
  * @param buffer A buffer to hold the result, i.e. the property value corresponding to the specified name.
  * @param defValue Value to return in case a property with the specified Name is not found in the Config Store
- * @return int8_t:\n
- *            - 1: If the Property was found in the store
- *            - 0: Otherwise
+ * @return size_t: Number of bytes written to the result buffer.
  */
-int8_t submitPropGetRequest(const std::string& prop, std::string& buffer, const std::string& defValue);
 
-/**
- * @brief Modifies an already existing property in the Config Store.
- * @details Note: This API is meant to be used internally i.e. by other Resource Tuner modules like Signals
- *          and not the End-Client Directly. Client Facing APIs are provided in Core/Client/APIs/
- * @param prop Name of the Property to be modified.
- * @param value A buffer holding the new the property value.
- * @return int8_t:\n
- *            - 1: If the Property with the specified name was found in the store, and was updated successfully.
- *            - 0: Otherwise
- */
-int8_t submitPropSetRequest(const std::string& prop, const std::string& value);
+size_t submitPropGetRequest(void* request, std::string& result);
 
-/**
- * @brief Submit an incoming Prop Request from a Client, for processing.
- * @details This API accepts both Prop Get and Set Requests from the Client, and processes them accordingly.
- * @param request Encapsulation type for the the Prop Request
- * @return int8_t:\n
- *            - 1: If the Property with the specified name was found in the store, and was updated successfully.
- *            - 0: Otherwise
- */
-ErrCode submitPropRequest(void* request);
+size_t submitPropGetRequest(const std::string& propName, std::string& result, const std::string& defVal);
 
 ErrCode translateToPhysicalIDs(Resource* resource);
 

resource-tuner/core/PropertiesRegistry.cpp

@@ -16,21 +16,21 @@ int8_t PropertiesRegistry::createProperty(const std::string& propertyName, const
     return true;
 }
 
-int8_t PropertiesRegistry::queryProperty(const std::string& propertyName, std::string& result) {
+size_t PropertiesRegistry::queryProperty(const std::string& propertyName, std::string& result) {
     if(propertyName.length() == 0) {
-        return false;
+        return 0;
     }
 
     this->mPropRegistryMutex.lock_shared();
     if(this->mProperties.find(propertyName) == this->mProperties.end()) {
         this->mPropRegistryMutex.unlock_shared();
-        return false;
+        return 0;
     }
 
     result = this->mProperties[propertyName];
     this->mPropRegistryMutex.unlock_shared();
 
-    return true;
+    return result.length();
 }
 
 int8_t PropertiesRegistry::modifyProperty(const std::string& propertyName, const std::string& propertyValue) {

resource-tuner/core/RequestHandler.cpp

@@ -374,27 +374,54 @@ void submitResProvisionReqMsg(void* msg) {
     }
 }
 
-int8_t submitPropGetRequest(const std::string& prop,
-                            std::string& buffer,
-                            const std::string& defaultValue) {
-    std::string propertyName(prop);
-    std::string result = "";
-
-    int8_t propFound = false;
-    if((propFound = PropertiesRegistry::getInstance()->queryProperty(propertyName, result)) == false) {
-        result = defaultValue;
+size_t submitPropGetRequest(void* msg, std::string& result) {
+    if(msg == nullptr) {
+        return 0;
     }
+    std::shared_ptr<PropertiesRegistry> propReg = PropertiesRegistry::getInstance();
 
-    buffer = result;
-    return propFound;
+    MsgForwardInfo* info = (MsgForwardInfo*) msg;
+    if(info == nullptr) {
+        return 0;
+    }
+
+    int8_t* ptr8 = (int8_t*)info->mBuffer;
+    DEREF_AND_INCR(ptr8, int8_t);
+    DEREF_AND_INCR(ptr8, int8_t);
+
+    uint64_t* ptr64 = (uint64_t*)ptr8;
+    DEREF_AND_INCR(ptr64, uint64_t);
+
+    char* charIterator = (char*)ptr64;
+    const char* propNamePtr = charIterator;
+
+    while(*charIterator != '\0') {
+        charIterator++;
+    }
+    charIterator++;
+    std::string propName = propNamePtr;
+
+    std::string buffer = "";
+    size_t writtenBytes = propReg->queryProperty(propName, buffer);
+    if(writtenBytes > 0) {
+        result = buffer;
+    }
+
+    return writtenBytes;
 }
 
-ErrCode submitPropRequest(void* context) {
-    if(context == nullptr) return RC_BAD_ARG;
-    PropConfig* propConfig = static_cast<PropConfig*>(context);
+size_t submitPropGetRequest(const std::string& propName,
+                            std::string& result,
+                            const std::string& defVal) {
+    std::shared_ptr<PropertiesRegistry> propReg = PropertiesRegistry::getInstance();
 
-    const char* defaultValue = "na";
-    submitPropGetRequest(propConfig->mPropName, propConfig->mResult, defaultValue);
+    std::string buffer = "";
+    size_t writtenBytes = propReg->queryProperty(propName, buffer);
+    if(writtenBytes > 0) {
+        result = buffer;
+    } else {
+        result = defVal;
+    }
 
-    return RC_SUCCESS;
+    return writtenBytes;
 }

resource-tuner/core/ResourceHooks.cpp

@@ -682,50 +682,12 @@ static void resetRunOnCoresExclusively(void* context) {
     }
 }
 
-static void setPmQos(void* context) {
-    if(context == nullptr) return;
-    Resource* resource = static_cast<Resource*>(context);
-
-    if(resource->getValuesCount() < 1) return;
-
-    int32_t clusterID = resource->getClusterValue();
-
-    ClusterInfo* cinfo = TargetRegistry::getInstance()->getClusterInfo(clusterID);
-    if(cinfo == nullptr) {
-        return;
-    }
-
-    for(int32_t i = cinfo->mStartCpu; i < cinfo->mStartCpu + cinfo->mNumCpus; i++) {
-        defaultCoreLevelApplierHelper(resource, i);
-    }
-}
-
-static void resetPmQos(void* context) {
-    if(context == nullptr) return;
-    Resource* resource = static_cast<Resource*>(context);
-
-    if(resource->getValuesCount() < 1) return;
-
-    int32_t clusterID = resource->getClusterValue();
-
-    ClusterInfo* cinfo = TargetRegistry::getInstance()->getClusterInfo(clusterID);
-    if(cinfo == nullptr) {
-        return;
-    }
-
-    for(int32_t i = cinfo->mStartCpu; i < cinfo->mStartCpu + cinfo->mNumCpus; i++) {
-        defaultCoreLevelTearHelper(resource, i);
-    }
-}
-
 // Register the specific Callbacks
-URM_REGISTER_RES_APPLIER_CB(0x00010001, setPmQos);
 URM_REGISTER_RES_APPLIER_CB(0x00090000, moveProcessToCGroup);
 URM_REGISTER_RES_APPLIER_CB(0x00090001, moveThreadToCGroup);
 URM_REGISTER_RES_APPLIER_CB(0x00090002, setRunOnCores);
 URM_REGISTER_RES_APPLIER_CB(0x00090003, setRunOnCoresExclusively);
 URM_REGISTER_RES_APPLIER_CB(0x00090005, limitCpuTime);
-URM_REGISTER_RES_TEAR_CB(0x00010001, resetPmQos);
 URM_REGISTER_RES_TEAR_CB(0x00090000, removeProcessFromCGroup);
 URM_REGISTER_RES_TEAR_CB(0x00090001, removeThreadFromCGroup);
 URM_REGISTER_RES_TEAR_CB(0x00090003, resetRunOnCoresExclusively);

resource-tuner/core/Server/RequestReceiver.cpp

@@ -15,6 +15,20 @@ void RequestReceiver::forwardMessage(int32_t clientSocket, MsgForwardInfo* info)
     info->mModuleID = moduleID;
     info->mRequestType = requestType;
 
+    if(info->mRequestType == REQ_PROP_GET) {
+        std::string result = "";
+        size_t writeLen = submitPropGetRequest(info, result);
+        if(writeLen == 0) {
+            result = "na";
+            writeLen = result.length();
+        }
+
+        if(write(clientSocket, result.c_str(), writeLen) == -1) {
+            TYPELOGV(ERRNO_LOG, "write", strerror(errno));
+        }
+        return;
+    }
+
     info->mHandle = AuxRoutines::generateUniqueHandle();
     if(info->mHandle < 0) {
         // Handle Generation Failure

resource-tuner/init/RestuneInit.cpp

@@ -707,4 +707,4 @@ static ErrCode tear(void* arg) {
     return RC_SUCCESS;
 }
 
-RESTUNE_REGISTER_MODULE(MOD_RESTUNE, init, tear);
+URM_REGISTER_MODULE(MOD_RESTUNE, init, tear);

tests/Integration/IntegrationTests.cpp

@@ -79,7 +79,7 @@ URM_TEST(TestHandleGeneration, {
     E_ASSERT((handle > 0));
 })
 
-URM_TEST_P(TestPropFetch, BROKEN, {
+URM_TEST(TestPropFetch, {
     char prop1[] = "resource_tuner.pulse.duration";
     char buf[64];
     memset(buf, 0, sizeof(buf));
@@ -100,7 +100,7 @@ URM_TEST_P(TestPropFetch, BROKEN, {
     E_ASSERT((std::string(buf) == "60"));
 
     // Non Existent
-    char prop3[] = "resource_tuner.benchmakr.comparison.utilinets";
+    char prop3[] = "resource_tuner.benchmark.comparison.utilinets";
     memset(buf, 0, sizeof(buf));
 
     status = getProp(prop3, buf, sizeof(buf), "na");