Race, USB and TCP improvements

CMakeLists.txt

@@ -16,6 +16,7 @@ option(XLINK_BUILD_EXAMPLES "Build XLink examples" OFF)
 option(XLINK_BUILD_TESTS "Build XLink tests" OFF)
 
 add_library(${TARGET_NAME} ${XLINK_SOURCES})
+add_default_flags(${TARGET_NAME})
 add_flag_source(src/shared/XLinkDevice.c "-Werror=switch-enum")
 
 if(WIN32)

cmake/Flags.cmake

@@ -18,7 +18,7 @@ endfunction()
 function(add_default_flags target)
     if ("${CMAKE_C_COMPILER_ID}" MATCHES "^(AppleClang|Clang|GNU)$")
         # enable those flags
-        add_flag(${target} -Woverloaded-virtual)     # warn if you overload (not override) a virtual function
+        # add_flag(${target} -Woverloaded-virtual)     # warn if you overload (not override) a virtual function
         add_flag(${target} -Wformat=2)               # warn on security issues around functions that format output (ie printf)
         add_flag(${target} -Wmisleading-indentation) # (only in GCC >= 6.0) warn if indentation implies blocks where blocks do not exist
         add_flag(${target} -Wduplicated-cond)        # (only in GCC >= 6.0) warn if if / else chain has duplicated conditions
@@ -37,9 +37,9 @@ function(add_default_flags target)
         add_flag(${target} -Werror=self-assign-field)  # error if self assign - bugprone
         add_flag(${target} -Werror=unused-lambda-capture)  # error if lambda capture is unused
         add_flag(${target} -Werror=return-type)      # warning: control reaches end of non-void function [-Wreturn-type]
-        add_flag(${target} -Werror=non-virtual-dtor) # warn the user if a class with virtual functions has a non-virtual destructor. This helps catch hard to track down memory errors
+        # add_flag(${target} -Werror=non-virtual-dtor) # warn the user if a class with virtual functions has a non-virtual destructor. This helps catch hard to track down memory errors
         add_flag(${target} -Werror=sign-compare)     # warn the user if they compare a signed and unsigned numbers
-        add_flag(${target} -Werror=reorder)          # field '$1' will be initialized after field '$2'
+        # add_flag(${target} -Werror=reorder)          # field '$1' will be initialized after field '$2'
 
     elseif ("${CMAKE_C_COMPILER_ID}" STREQUAL "MSVC")
         # using Visual Studio C++

include/XLink/XLink.h

@@ -74,6 +74,15 @@ XLinkError_t XLinkFindAllSuitableDevices(XLinkDeviceState_t state,
  */
 XLinkError_t XLinkConnect(XLinkHandler_t* handler);
 
+/**
+ * TODO(themarpe) - doesn't work well yet...
+ * @brief Connects to specific device with a timeout, starts dispatcher and pings remote
+ * @param[in,out] handler - XLink communication parameters (file path name for underlying layer)
+ * @param[in] msTimeout – time in milliseconds after which operation times out
+ * @return Status code of the operation: X_LINK_SUCCESS (0) for success
+XLinkError_t XLinkConnectWithTimeout(XLinkHandler_t* handler, const unsigned int msTimeout);
+*/
+
 /**
  * @brief Puts device into bootloader mode
  * @param deviceDesc - device description structure, obtained from XLinkFind* functions call
@@ -114,22 +123,17 @@ XLinkError_t XLinkBootFirmware(const deviceDesc_t* deviceDesc, const char* firmw
  * @return Status code of the operation: X_LINK_SUCCESS (0) for success
  */
 
-XLinkError_t XLinkResetRemote(linkId_t id);
+XLinkError_t XLinkResetRemote(const linkId_t id);
 
 /**
  * @brief Resets the remote device and close all open local handles for this device
  * @warning This function should be used in a host application
  * @param[in] id - link Id obtained from XLinkConnect in the handler parameter
+ * @param[in] msTimeout – time in milliseconds after which operation times out
  * @return Status code of the operation: X_LINK_SUCCESS (0) for success
  */
 
-XLinkError_t XLinkResetRemoteTimeout(linkId_t id, int timeoutMs);
-
-/**
- * @brief Closes all and release all memory
- * @return Status code of the operation: X_LINK_SUCCESS (0) for success
- */
-XLinkError_t XLinkResetAll();
+XLinkError_t XLinkResetRemoteTimeout(const linkId_t id, const unsigned int msTimeout);
 
 /**
  * @brief Retrieves USB speed of certain connected device
@@ -303,10 +307,10 @@ XLinkError_t XLinkReleaseData(streamId_t const streamId);
  *      data. Should be fixed for the next release.
  * @param[in]   streamId – stream link Id obtained from XLinkOpenStream call
  * @param[out]  packet – structure containing output data buffer and received size
- * @param[in]   timeoutMs – timeout for a read operation in milliseconds
+ * @param[in]   msTimeout – timeout for a read operation in milliseconds
  * @return Status code of the operation: X_LINK_SUCCESS (0) for success
  */
-XLinkError_t XLinkReadDataWithTimeout(streamId_t streamId, streamPacketDesc_t** packet, unsigned int timeoutMs);
+XLinkError_t XLinkReadDataWithTimeout(streamId_t streamId, streamPacketDesc_t** packet, unsigned int msTimeout);
 
 /**
  * @brief Sends a package to initiate the writing of data to a remote stream with timeout in ms
@@ -315,10 +319,10 @@ XLinkError_t XLinkReadDataWithTimeout(streamId_t streamId, streamPacketDesc_t**
  * @param[in] streamId – stream link Id obtained from XLinkOpenStream call
  * @param[in] buffer – data buffer to be transmitted
  * @param[in] size – size of the data to be transmitted
- * @param[in] timeoutMs – timeout for a write operation in milliseconds
+ * @param[in] msTimeout – timeout for a write operation in milliseconds
  * @return Status code of the operation: X_LINK_SUCCESS (0) for success
  */
-XLinkError_t XLinkWriteDataWithTimeout(streamId_t streamId, const uint8_t* buffer, int size, unsigned int timeoutMs);
+XLinkError_t XLinkWriteDataWithTimeout(streamId_t streamId, const uint8_t* buffer, int size, unsigned int msTimeout);
 
 // ------------------------------------
 // Device streams management. End.
@@ -346,8 +350,12 @@ XLinkError_t XLinkAsyncWriteData();
 XLinkError_t XLinkSetDeviceOpenTimeOutMsec(unsigned int msec);
 XLinkError_t XLinkSetCommonTimeOutMsec(unsigned int msec);
 
-// unsafe
-XLinkError_t XLinkGetFillLevel(streamId_t const streamId, int isRemote, int* fillLevel);
+/**
+ * Deprecated - issues
+ * @brief Closes all and release all memory
+ * @return Status code of the operation: X_LINK_SUCCESS (0) for success
+ */
+XLinkError_t XLinkResetAll();
 
 #endif // __PC__
 

include/XLink/XLinkPrivateFields.h

@@ -40,8 +40,10 @@ extern sem_t  pingSem; //to b used by myriad
 
 xLinkDesc_t* getLinkById(linkId_t id);
 xLinkDesc_t* getLink(void* fd);
+xLinkDesc_t* getLinkUnsafe(void* fd);
 xLinkState_t getXLinkState(xLinkDesc_t* link);
-
+XLinkError_t getLinkUpDeviceHandleByLinkId(linkId_t const linkId, xLinkDeviceHandle_t* const out_handle);
+XLinkError_t getLinkUpDeviceHandleByStreamId(streamId_t const streamId, xLinkDeviceHandle_t* const out_handle);
 
 streamId_t getStreamIdByName(xLinkDesc_t* link, const char* name);
 

src/pc/PlatformData.c

@@ -169,7 +169,7 @@ void XLinkPlatformDeallocateData(void *ptr, uint32_t size, uint32_t alignment)
 // Wrappers implementation. Begin.
 // ------------------------------------
 
-int usbPlatformRead(void* fd, void* data, int size)
+int usbPlatformRead(void* fdKey, void* data, int size)
 {
     int rc = 0;
 #ifndef USE_USB_VSC
@@ -209,12 +209,20 @@ int usbPlatformRead(void* fd, void* data, int size)
     }
 #endif  /*USE_LINK_JTAG*/
 #else
-    rc = usb_read((libusb_device_handle *) fd, data, size);
+
+    void* tmpUsbHandle = NULL;
+    if(getPlatformDeviceFdFromKey(fdKey, &tmpUsbHandle)){
+        mvLog(MVLOG_FATAL, "Cannot find file descriptor by key: %" PRIxPTR, (uintptr_t) fdKey);
+        return -1;
+    }
+    libusb_device_handle* usbHandle = (libusb_device_handle*) tmpUsbHandle;
+
+    rc = usb_read(usbHandle, data, size);
 #endif  /*USE_USB_VSC*/
     return rc;
 }
 
-int usbPlatformWrite(void *fd, void *data, int size)
+int usbPlatformWrite(void *fdKey, void *data, int size)
 {
     int rc = 0;
 #ifndef USE_USB_VSC
@@ -257,7 +265,15 @@ int usbPlatformWrite(void *fd, void *data, int size)
     }
 #endif  /*USE_LINK_JTAG*/
 #else
-    rc = usb_write((libusb_device_handle *) fd, data, size);
+
+    void* tmpUsbHandle = NULL;
+    if(getPlatformDeviceFdFromKey(fdKey, &tmpUsbHandle)){
+        mvLog(MVLOG_FATAL, "Cannot find file descriptor by key: %" PRIxPTR, (uintptr_t) fdKey);
+        return -1;
+    }
+    libusb_device_handle* usbHandle = (libusb_device_handle*) tmpUsbHandle;
+
+    rc = usb_write(usbHandle, data, size);
 #endif  /*USE_USB_VSC*/
     return rc;
 }
@@ -277,7 +293,7 @@ int pciePlatformWrite(void *f, void *data, int size)
     {
         write_pending = 1;
 
-        size_t chunk = size < CHUNK_SIZE_BYTES ? size : CHUNK_SIZE_BYTES;
+        size_t chunk = (size_t)size < CHUNK_SIZE_BYTES ? (size_t)size : CHUNK_SIZE_BYTES;
         int num_written = pcie_write(f, data, chunk);
 
         write_pending = 0;

src/pc/PlatformDeviceControl.c

@@ -47,7 +47,7 @@ int usbFdRead = -1;
 static UsbSpeed_t usb_speed_enum = X_LINK_USB_SPEED_UNKNOWN;
 static char mx_serial[XLINK_MAX_MX_ID_SIZE] = { 0 };
 #ifdef USE_USB_VSC
-static int statuswaittimeout = 5;
+static const int statuswaittimeout = 5;
 #endif
 
 typedef struct {
@@ -617,18 +617,21 @@ int usbPlatformConnect(const char *devPathRead, const char *devPathWrite, void *
     return 0;
 #endif  /*USE_LINK_JTAG*/
 #else
-    *fd = usbLinkOpen(devPathWrite);
-    if (*fd == 0)
+    void* usbHandle = usbLinkOpen(devPathWrite);
+
+    if (usbHandle == 0)
     {
         /* could fail due to port name change */
         return -1;
     }
 
-    if(*fd)
-        return 0;
-    else
-        return -1;
+    // Store the usb handle and create a "unique" key instead
+    // (as file descriptors are reused and can cause a clash with lookups between scheduler and link)
+    *fd = createPlatformDeviceFdKey(usbHandle);
+
 #endif  /*USE_USB_VSC*/
+
+    return 0;
 }
 
 int pciePlatformConnect(UNUSED const char *devPathRead,
@@ -642,14 +645,23 @@ int pciePlatformConnect(UNUSED const char *devPathRead,
 int tcpipPlatformConnect(const char *devPathRead, const char *devPathWrite, void **fd)
 {
 #if defined(USE_TCP_IP)
-    if (!devPathWrite || !fd)
+    if (!devPathWrite || !fd) {
         return X_LINK_PLATFORM_INVALID_PARAMETERS;
+    }
+
     TCPIP_SOCKET sock = socket(AF_INET, SOCK_STREAM, 0);
+
+#if (defined(_WIN32) || defined(_WIN64) )
+    if(sock == INVALID_SOCKET)
+    {
+        return TCPIP_HOST_ERROR;
+    }
+#else
     if(sock < 0)
     {
-        tcpip_close_socket(sock);
-        return -1;
+        return TCPIP_HOST_ERROR;
     }
+#endif
 
     // Disable sigpipe reception on send
     #if defined(SO_NOSIGPIPE)
@@ -724,7 +736,7 @@ int tcpipPlatformBootBootloader(const char *name)
     return tcpip_boot_bootloader(name);
 }
 
-int usbPlatformClose(void *fd)
+int usbPlatformClose(void *fdKey)
 {
 
 #ifndef USE_USB_VSC
@@ -741,7 +753,19 @@ int usbPlatformClose(void *fd)
     }
 #endif  /*USE_LINK_JTAG*/
 #else
-    usbLinkClose((libusb_device_handle *) fd);
+
+    void* tmpUsbHandle = NULL;
+    if(getPlatformDeviceFdFromKey(fdKey, &tmpUsbHandle)){
+        mvLog(MVLOG_FATAL, "Cannot find USB Handle by key");
+        return -1;
+    }
+    usbLinkClose((libusb_device_handle *) tmpUsbHandle);
+
+    if(destroyPlatformDeviceFdKey(fdKey)){
+        mvLog(MVLOG_FATAL, "Cannot destroy USB Handle key");
+        return -1;
+    }
+
 #endif  /*USE_USB_VSC*/
     return -1;
 }
@@ -794,7 +818,7 @@ int tcpipPlatformClose(void *fdKey)
 #endif
 
     if(destroyPlatformDeviceFdKey(fdKey)){
-        mvLog(MVLOG_FATAL, "Cannot destory file descriptor key");
+        mvLog(MVLOG_FATAL, "Cannot destroy file descriptor key");
         return -1;
     }
 

src/pc/Win/src/win_semaphore.c

@@ -94,10 +94,15 @@ int sem_trywait(sem_t *sem){
         return ls_set_errno(EINVAL);
     }
     sem_t s = *sem;
-    if (WaitForSingleObject(s->handle, 0) != WAIT_OBJECT_0) {
+
+    DWORD ret = WaitForSingleObject(s->handle, 0);
+    if (ret == WAIT_OBJECT_0) {
+        return 0;
+    } else if(ret == WAIT_TIMEOUT) {
+        return ls_set_errno(ETIMEDOUT);
+    } else {
         return ls_set_errno(EINVAL);
     }
-    return 0;
 }