chore: increase coverage in n_request.c

Author: zfields

n_i2c.c

@@ -301,7 +301,7 @@ bool _i2cNoteReset(void)
             break;
         }
 
-        NOTE_C_LOG_DEBUG("retrying I2C interface reset...")
+        NOTE_C_LOG_DEBUG("retrying I2C interface reset...");
     }
 
     // Done with the I2C bus

n_request.c

@@ -36,6 +36,57 @@ NOTE_C_STATIC bool _crcError(char *json, uint16_t shouldBeSeqno);
 NOTE_C_STATIC bool notecardFirmwareSupportsCrc = false;
 #endif // !NOTE_C_LOW_MEM
 
+/*!
+ @internal
+
+ @brief Create a JSON object containing an error message.
+
+ Create a dynamically allocated `J` object containing a single string field
+ "err" whose value is the passed in error message.
+
+ @param id The "id" from the original request that resulted in an error
+ @param errmsg The error message.
+
+ @returns A `J` object with the "err" field populated.
+ */
+NOTE_C_STATIC J * _errDoc(uint32_t id, const char *errmsg)
+{
+    J *rspdoc = JCreateObject();
+    if (rspdoc != NULL) {
+        JAddStringToObject(rspdoc, c_err, errmsg);
+        JAddStringToObject(rspdoc, "src", "note-c");
+        if (id) {
+            JAddIntToObject(rspdoc, "id", id);
+        }
+        if (suppressShowTransactions == 0) {
+            _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, "[ERROR] ");
+            _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, "{\"err\":\"");
+            _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, errmsg);
+            _DebugWithLevelLn(NOTE_C_LOG_LEVEL_ERROR, "\",\"src\":\"note-c\"}");
+        }
+    } else {
+        NOTE_C_LOG_ERROR("Failed to allocate error document!");
+    }
+
+    return rspdoc;
+}
+
+/*!
+ @brief Resume showing transaction details.
+ */
+void _noteResumeTransactionDebug(void)
+{
+    suppressShowTransactions--;
+}
+
+/*!
+ @brief Suppress showing transaction details.
+ */
+void _noteSuspendTransactionDebug(void)
+{
+    suppressShowTransactions++;
+}
+
 /*!
  @internal
 
@@ -92,55 +143,6 @@ NOTE_C_STATIC bool notecardFirmwareSupportsCrc = false;
     return result;
 }
 
-/*!
- @internal
-
- @brief Create a JSON object containing an error message.
-
- Create a dynamically allocated `J` object containing a single string field
- "err" whose value is the passed in error message.
-
- @param id The "id" from the original request that resulted in an error
- @param errmsg The error message.
-
- @returns A `J` object with the "err" field populated.
- */
-NOTE_C_STATIC J * errDoc(uint32_t id, const char *errmsg)
-{
-    J *rspdoc = JCreateObject();
-    if (rspdoc != NULL) {
-        JAddStringToObject(rspdoc, c_err, errmsg);
-        JAddStringToObject(rspdoc, "src", "note-c");
-        if (id) {
-            JAddIntToObject(rspdoc, "id", id);
-        }
-        if (suppressShowTransactions == 0) {
-            _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, "[ERROR] ");
-            _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, "{\"err\":\"");
-            _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, errmsg);
-            _DebugWithLevelLn(NOTE_C_LOG_LEVEL_ERROR, "\",\"src\":\"note-c\"}");
-        }
-    }
-
-    return rspdoc;
-}
-
-/*!
- @brief Suppress showing transaction details.
- */
-void _noteSuspendTransactionDebug(void)
-{
-    suppressShowTransactions++;
-}
-
-/*!
- @brief Resume showing transaction details.
- */
-void _noteResumeTransactionDebug(void)
-{
-    suppressShowTransactions--;
-}
-
 /*!
  @brief Suppress showing transaction details.
  */
@@ -367,11 +369,18 @@ J *NoteRequestResponseWithRetry(J *req, uint32_t timeoutSeconds)
           if there was no response or if there was an error.
 
  @note When a "cmd" is sent, it is not possible to determine if an error occurred.
+
+ @note Unlike the `NoteRequest*` functions, this function does not automatically
+       free the request JSON string. It is not possible to know if the parameter
+       is a string literal. As such, it is the caller's responsibility to manage
+       the memory associated with the request string.
 */
 char * NoteRequestResponseJSON(const char *reqJSON)
 {
     const uint32_t transactionTimeoutMs = (CARD_INTER_TRANSACTION_TIMEOUT_SEC * 1000);
     char *rspJSON = NULL;
+    char *allocatedJSON = NULL; // required to free the string if it is not newline-terminated
+    bool isCmdPipeline = false;
 
     if (reqJSON == NULL) {
         return NULL;
@@ -396,25 +405,26 @@ char * NoteRequestResponseJSON(const char *reqJSON)
             // All JSON strings should be newline-terminated to meet the
             // specification, however this is required to ensure backward
             // compatibility with the previous implementation.
-            const size_t tempLen = strlen(reqJSON);
-            if (0 == tempLen) {
+            const size_t allocLen = strlen(reqJSON);
+            if (0 == allocLen) {
                 NOTE_C_LOG_ERROR(ERRSTR("request: jsonbuf zero length", c_bad));
                 break;
             }
 
             NOTE_C_LOG_WARN(ERRSTR("Memory allocation due to malformed request (not newline-terminated)", c_bad));
-            char * const temp = _Malloc(tempLen + 2);  // +2 for newline and null-terminator
-            if (temp == NULL) {
+            allocatedJSON = _Malloc(allocLen + 2);  // +2 for newline and null-terminator
+            if (allocatedJSON == NULL) {
                 NOTE_C_LOG_ERROR(ERRSTR("request: jsonbuf malloc failed", c_mem));
                 break;
             }
 
-            memcpy(temp, reqJSON, tempLen);
-            temp[tempLen] = '\n';
-            temp[tempLen + 1] = '\0';
-            reqJSON = temp;
-            endPtr = &temp[tempLen];
+            memcpy(allocatedJSON, reqJSON, allocLen);
+            allocatedJSON[allocLen] = '\n';
+            allocatedJSON[allocLen + 1] = '\0';
+            reqJSON = allocatedJSON;
+            endPtr = &allocatedJSON[allocLen];
         } else {
+            isCmdPipeline = !(strlen(newlinePtr) == 1);
             endPtr = newlinePtr;
         }
         const size_t reqLen = ((endPtr - reqJSON) + 1);
@@ -439,15 +449,17 @@ char * NoteRequestResponseJSON(const char *reqJSON)
             const char *errstr = _Transaction(reqJSON, reqLen, &rspJSON, transactionTimeoutMs);
             if (errstr != NULL) {
                 NOTE_C_LOG_ERROR(errstr);
+
+                // Extract ID from the request JSON, if present
                 uint32_t id = 0;
-                if (reqJSON != NULL) {
-                    J *req = JParse(reqJSON);
-                    if (req != NULL) {
-                        id = JGetInt(req, "id");
-                        JDelete(req);
-                    }
+                J *req = JParse(reqJSON);
+                if (req != NULL) {
+                    id = JGetInt(req, "id");
+                    JDelete(req);
                 }
-                J *errdoc = errDoc(id, errstr);
+
+                // Use _errDoc() to create a well-formed JSON error string
+                J *errdoc = _errDoc(id, errstr);
                 if (errdoc != NULL) {
                     char *errdocJSON = JPrintUnformatted(errdoc);
                     JDelete(errdoc);
@@ -463,25 +475,31 @@ char * NoteRequestResponseJSON(const char *reqJSON)
                     }
                 }
             }
-            if (NULL == newlinePtr) {
-                _Free((void *)reqJSON);
+            if (allocatedJSON) {
+                _Free((void *)allocatedJSON);
+                allocatedJSON = NULL;
             }
             break;
         } else {
             // If it's a command, the Notecard will not respond, so we pass
             // NULL for the response parameter.
             const char *errstr = _Transaction(reqJSON, reqLen, NULL, transactionTimeoutMs);
-            reqJSON = (endPtr + 1);
+            reqJSON = (endPtr + 1);  // Move to the next command in the pipeline
             if (errstr != NULL) {
                 NOTE_C_LOG_ERROR(errstr);
             }
         }
 
         // Clean up if we allocated a new string
-        if (NULL == newlinePtr) {
-            _Free((void *)reqJSON);
+        (void)isCmdPipeline;
+        if (allocatedJSON) {
+            _Free((void *)allocatedJSON);
+            break;
         }
-    }
+        if (!isCmdPipeline) {
+            break;
+        }
+    } // for(;;)
 
     _UnlockNote();
     _TransactionStop();
@@ -567,7 +585,7 @@ J *_noteTransactionShouldLock(J *req, bool lockNotecard)
             NOTE_C_LOG_ERROR(errStr);
             return NULL;
         }
-        return errDoc(id, errStr);
+        return _errDoc(id, errStr);
     }
 
     // Inject the user agent object only when we're doing a `hub.set` and
@@ -596,7 +614,7 @@ J *_noteTransactionShouldLock(J *req, bool lockNotecard)
                 NOTE_C_LOG_ERROR(errStr);
                 return NULL;
             }
-            return errDoc(id, errStr);
+            return _errDoc(id, errStr);
         }
     }
 
@@ -788,7 +806,7 @@ J *_noteTransactionShouldLock(J *req, bool lockNotecard)
             rsp = NULL;
         }
         NoteResetRequired(); // queue up a reset
-        J *errRsp = errDoc(id, errStr);
+        J *errRsp = _errDoc(id, errStr);
         if (lockNotecard) {
             _UnlockNote();
         }

n_serial.c

@@ -190,7 +190,7 @@ bool _serialNoteReset(void)
             return false;
         }
 
-        NOTE_C_LOG_DEBUG("retrying Serial interface reset.")
+        NOTE_C_LOG_DEBUG("retrying Serial interface reset.");
     }
 
     // Done

note.h

@@ -72,6 +72,7 @@ enum {
 
 #ifdef NOTE_C_LOW_MEM
 #define ERRSTR(x,y) (y)
+#define NOTE_C_LOG_DEBUG(msg) do { } while (0)
 #else
 #define ERRSTR(x,y) (x)
 #endif // NOTE_C_LOW_MEM
@@ -384,29 +385,37 @@ void NoteDebugWithLevelLn(uint8_t level, const char *msg);
 #define NOTE_C_LOG_FILE_AND_LINE(lvl)
 #endif
 
+#ifndef NOTE_C_LOG_ERROR
 #define NOTE_C_LOG_ERROR(msg) do { \
   NOTE_C_LOG_FILE_AND_LINE(NOTE_C_LOG_LEVEL_ERROR); \
   NoteDebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, "[ERROR] "); \
   NoteDebugWithLevelLn(NOTE_C_LOG_LEVEL_ERROR, msg); \
-} while (0);
+} while (0)
+#endif
 
+#ifndef NOTE_C_LOG_WARN
 #define NOTE_C_LOG_WARN(msg) do { \
   NOTE_C_LOG_FILE_AND_LINE(NOTE_C_LOG_LEVEL_WARN); \
   NoteDebugWithLevel(NOTE_C_LOG_LEVEL_WARN, "[WARN] "); \
   NoteDebugWithLevelLn(NOTE_C_LOG_LEVEL_WARN, msg); \
-} while (0);
+} while (0)
+#endif
 
+#ifndef NOTE_C_LOG_INFO
 #define NOTE_C_LOG_INFO(msg) do { \
   NOTE_C_LOG_FILE_AND_LINE(NOTE_C_LOG_LEVEL_INFO); \
   NoteDebugWithLevel(NOTE_C_LOG_LEVEL_INFO, "[INFO] "); \
   NoteDebugWithLevelLn(NOTE_C_LOG_LEVEL_INFO, msg); \
-} while (0);
+} while (0)
+#endif
 
+#ifndef NOTE_C_LOG_DEBUG
 #define NOTE_C_LOG_DEBUG(msg) do { \
   NOTE_C_LOG_FILE_AND_LINE(NOTE_C_LOG_LEVEL_DEBUG); \
   NoteDebugWithLevel(NOTE_C_LOG_LEVEL_DEBUG, "[DEBUG] "); \
   NoteDebugWithLevelLn(NOTE_C_LOG_LEVEL_DEBUG, msg); \
-} while (0);
+} while (0)
+#endif
 
 // The default log level
 #define NOTE_C_LOG_LEVEL_DEFAULT NOTE_C_LOG_LEVEL_INFO

test/CMakeLists.txt

@@ -55,6 +55,7 @@ endmacro(add_test)
 
 add_test(_crcAdd_test)
 add_test(_crcError_test)
+add_test(_errDoc_test)
 add_test(_i2cChunkedReceive_test)
 add_test(_i2cChunkedTransmit_test)
 add_test(_i2cNoteChunkedReceive_test)
@@ -75,6 +76,7 @@ add_test(_noteSerialReceive_test)
 add_test(_noteSerialReset_test)
 add_test(_noteSerialTransmit_test)
 add_test(_noteSetActiveInterface_test)
+add_test(_noteTransaction_calculateTimeoutMs_test)
 add_test(_serialChunkedReceive_test)
 add_test(_serialChunkedTransmit_test)
 add_test(_serialNoteReset_test)
@@ -168,6 +170,7 @@ add_test(NoteRequestResponseWithRetry_test)
 add_test(NoteRequestWithRetry_test)
 add_test(NoteReset_test)
 add_test(NoteResponseError_test)
+add_test(NoteResumeTransactionDebug_test)
 add_test(NoteSendToRoute_test)
 add_test(NoteSerialHooks_test)
 add_test(NoteSetActiveInterface_test)
@@ -194,6 +197,7 @@ add_test(NoteSetSerialNumber_test)
 add_test(NoteSetSyncMode_test)
 add_test(NoteSetUploadMode_test)
 add_test(NoteSleep_test)
+add_test(NoteSuspendTransactionDebug_test)
 add_test(NoteTemplate_test)
 add_test(NoteTime_test)
 add_test(NoteTimeSet_test)

test/include/test_static.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <stdint.h>
+
 #include "n_lib.h"
 
 // If we're building the tests, NOTE_C_STATIC is defined to nothing. This allows
@@ -15,11 +17,14 @@ extern "C" {
 char *_crcAdd(char *json, uint16_t seqno);
 bool _crcError(char *json, uint16_t shouldBeSeqno);
 void _delayIO(void);
+J * _errDoc(uint32_t id, const char *errmsg);
 const char * _i2cNoteQueryLength(uint32_t * available, uint32_t timeoutMs);
+char _j_tolower(char c);
 void _noteSetActiveInterface(int interface);
+uint32_t _noteTransaction_calculateTimeoutMs(J *req, bool isReq);
+unsigned char *_print(const J * const item, Jbool format, Jbool omitempty);
 void _setTime(JTIME seconds);
 bool _timerExpiredSecs(uint32_t *timer, uint32_t periodSecs);
-char _j_tolower(char c);
 
 #ifdef __cplusplus
 }