Skip to content

Fix several warnings and add some safety checks #342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Timac wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Fix several warnings and add some safety checks #342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
736b767
Fix 2 warnings due to los of integer precision
Jun 27, 2017
f66db43
Add __unused attribute to prevent warnings with unused parameters
Jun 27, 2017
1a7f30f
Ensure that the barrier parameter is valid.
Jun 27, 2017
3b5867a
Fix a warning caused by comparing 2 integers if different signs CFInd…
Jun 27, 2017
7ce48b0
Ensure that the context is valid
Jun 27, 2017
2a8afc0
Ensure that the hid_device pointer is valid
Jun 27, 2017
5401d6f
Set errno to EINVAL when the hid_device is not valid
Jun 27, 2017
94cbd98
Fix warnings caused by los of integer precision
Jun 27, 2017
5dcfa0e
Fix a potential issue where calloc could be called with an allocation…
Jun 27, 2017
986005b
Fix a possible NULL pointer dereference which would lead to a crash
Jun 27, 2017
8afab11
Fix a couple of warnings when building on Windows x64
Jul 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 104 additions & 13 deletions mac/hid.c
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,13 @@ typedef struct pthread_barrier {
int trip_count;
} pthread_barrier_t;

static int pthread_barrier_init(pthread_barrier_t *barrier, const pthread_barrierattr_t *attr, unsigned int count)
static int pthread_barrier_init(pthread_barrier_t *barrier, const pthread_barrierattr_t * __unused attr, unsigned int count)
{
if (!barrier) {
errno = EINVAL;
return -1;
}

if(count == 0) {
errno = EINVAL;
return -1;
Expand All @@ -69,13 +74,23 @@ static int pthread_barrier_init(pthread_barrier_t *barrier, const pthread_barrie

static int pthread_barrier_destroy(pthread_barrier_t *barrier)
{
if (!barrier) {
errno = EINVAL;
return -1;
}

pthread_cond_destroy(&barrier->cond);
pthread_mutex_destroy(&barrier->mutex);
return 0;
}

static int pthread_barrier_wait(pthread_barrier_t *barrier)
{
if (!barrier) {
errno = EINVAL;
return -1;
}

pthread_mutex_lock(&barrier->mutex);
++(barrier->count);
if(barrier->count >= barrier->trip_count)
Expand Down Expand Up @@ -249,7 +264,7 @@ static int get_string_property(IOHIDDeviceRef device, CFStringRef prop, wchar_t
len * sizeof(wchar_t),
&used_buf_len);

if (chars_copied == len)
if (chars_copied == (CFIndex)len)
buf[len] = 0; /* len is decremented above */
else
buf[chars_copied] = 0;
Expand Down Expand Up @@ -540,9 +555,14 @@ hid_device * HID_API_EXPORT hid_open(unsigned short vendor_id, unsigned short pr
return handle;
}

static void hid_device_removal_callback(void *context, IOReturn result,
void *sender)
static void hid_device_removal_callback(void *context, IOReturn __unused result,
void * __unused sender)
{
if (!context) {
errno = EINVAL;
return;
}

/* Stop the Run Loop for this device. */
hid_device *d = context;

Expand All @@ -553,13 +573,18 @@ static void hid_device_removal_callback(void *context, IOReturn result,
/* The Run Loop calls this function for each input report received.
This function puts the data into a linked list to be picked up by
hid_read(). */
static void hid_report_callback(void *context, IOReturn result, void *sender,
IOHIDReportType report_type, uint32_t report_id,
static void hid_report_callback(void *context, IOReturn __unused result, void * __unused sender,
IOHIDReportType __unused report_type, uint32_t __unused report_id,
uint8_t *report, CFIndex report_length)
{
struct input_report *rpt;
hid_device *dev = context;

if (!context) {
errno = EINVAL;
return;
}

/* Make a new Input Report object */
rpt = calloc(1, sizeof(struct input_report));
rpt->data = calloc(1, report_length);
Expand Down Expand Up @@ -605,6 +630,11 @@ static void hid_report_callback(void *context, IOReturn result, void *sender,
hid_close(), and serves to stop the read_thread's run loop. */
static void perform_signal_callback(void *context)
{
if (!context) {
errno = EINVAL;
return;
}

hid_device *dev = context;
CFRunLoopStop(dev->run_loop); /*TODO: CFRunLoopGetCurrent()*/
}
Expand All @@ -614,6 +644,11 @@ static void *read_thread(void *param)
hid_device *dev = param;
SInt32 code;

if (!dev) {
errno = EINVAL;
return NULL;
}

/* Move the device's run loop to this thread. */
IOHIDDeviceScheduleWithRunLoop(dev->device_handle, CFRunLoopGetCurrent(), dev->run_loop_mode);

Expand Down Expand Up @@ -710,6 +745,11 @@ hid_device * HID_API_EXPORT hid_open_path(const char *path)

/* Create the buffers for receiving data */
dev->max_input_report_len = (CFIndex) get_max_report_length(dev->device_handle);
if (dev->max_input_report_len <= 0) {
/* Error getting kIOHIDMaxInputReportSizeKey */
goto return_error;
}

dev->input_report_buf = calloc(dev->max_input_report_len, sizeof(uint8_t));

/* Create the Run Loop Mode for this device.
Expand Down Expand Up @@ -754,6 +794,11 @@ static int set_report(hid_device *dev, IOHIDReportType type, const unsigned char
size_t length_to_send;
IOReturn res;

if (!dev) {
errno = EINVAL;
return -1;
}

/* Return if the device has been disconnected. */
if (dev->disconnected)
return -1;
Expand All @@ -778,7 +823,7 @@ static int set_report(hid_device *dev, IOHIDReportType type, const unsigned char
data_to_send, length_to_send);

if (res == kIOReturnSuccess) {
return length;
return (int)length;
}
else
return -1;
Expand All @@ -795,19 +840,34 @@ int HID_API_EXPORT hid_write(hid_device *dev, const unsigned char *data, size_t
/* Helper function, so that this isn't duplicated in hid_read(). */
static int return_data(hid_device *dev, unsigned char *data, size_t length)
{
if (!dev) {
errno = EINVAL;
return 0;
}

/* Copy the data out of the linked list item (rpt) into the
return buffer (data), and delete the liked list item. */
struct input_report *rpt = dev->input_reports;
if(rpt == NULL) {
errno = EINVAL;
return 0;
}

size_t len = (length < rpt->len)? length: rpt->len;
memcpy(data, rpt->data, len);
dev->input_reports = rpt->next;
free(rpt->data);
free(rpt);
return len;
return (int)len;
}

static int cond_wait(const hid_device *dev, pthread_cond_t *cond, pthread_mutex_t *mutex)
{
if (!dev) {
errno = EINVAL;
return -1;
}

while (!dev->input_reports) {
int res = pthread_cond_wait(cond, mutex);
if (res != 0)
Expand All @@ -828,6 +888,11 @@ static int cond_wait(const hid_device *dev, pthread_cond_t *cond, pthread_mutex_

static int cond_timedwait(const hid_device *dev, pthread_cond_t *cond, pthread_mutex_t *mutex, const struct timespec *abstime)
{
if (!dev) {
errno = EINVAL;
return -1;
}

while (!dev->input_reports) {
int res = pthread_cond_timedwait(cond, mutex, abstime);
if (res != 0)
Expand All @@ -849,6 +914,11 @@ static int cond_timedwait(const hid_device *dev, pthread_cond_t *cond, pthread_m

int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t length, int milliseconds)
{
if (!dev) {
errno = EINVAL;
return -1;
}

int bytes_read = -1;

/* Lock the access to the report list. */
Expand Down Expand Up @@ -944,6 +1014,11 @@ int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned char *data,
CFIndex len = length;
IOReturn res;

if (!dev) {
errno = EINVAL;
return -1;
}

/* Return if the device has been unplugged. */
if (dev->disconnected)
return -1;
Expand All @@ -953,16 +1028,18 @@ int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned char *data,
data[0], /* Report ID */
data, &len);
if (res == kIOReturnSuccess)
return len;
return (int)len;
else
return -1;
}


void HID_API_EXPORT hid_close(hid_device *dev)
{
if (!dev)
return;
if (!dev) {
errno = EINVAL;
return;
}

/* Disconnect the report callback before close. */
if (!dev->disconnected) {
Expand Down Expand Up @@ -1007,28 +1084,42 @@ void HID_API_EXPORT hid_close(hid_device *dev)

int HID_API_EXPORT_CALL hid_get_manufacturer_string(hid_device *dev, wchar_t *string, size_t maxlen)
{
if (!dev) {
errno = EINVAL;
return -1;
}
return get_manufacturer_string(dev->device_handle, string, maxlen);
}

int HID_API_EXPORT_CALL hid_get_product_string(hid_device *dev, wchar_t *string, size_t maxlen)
{
if (!dev) {
errno = EINVAL;
return -1;
}

return get_product_string(dev->device_handle, string, maxlen);
}

int HID_API_EXPORT_CALL hid_get_serial_number_string(hid_device *dev, wchar_t *string, size_t maxlen)
{
if (!dev) {
errno = EINVAL;
return -1;
}

return get_serial_number(dev->device_handle, string, maxlen);
}

int HID_API_EXPORT_CALL hid_get_indexed_string(hid_device *dev, int string_index, wchar_t *string, size_t maxlen)
int HID_API_EXPORT_CALL hid_get_indexed_string(hid_device * __unused dev, int __unused string_index, wchar_t * __unused string, size_t __unused maxlen)
{
/* TODO: */

return 0;
}


HID_API_EXPORT const wchar_t * HID_API_CALL hid_error(hid_device *dev)
HID_API_EXPORT const wchar_t * HID_API_CALL hid_error(hid_device * __unused dev)
{
/* TODO: */

Expand Down
22 changes: 11 additions & 11 deletions windows/hid.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ int HID_API_EXPORT HID_API_CALL hid_write(hid_device *dev, const unsigned char *
length = dev->output_report_length;
}

res = WriteFile(dev->device_handle, buf, length, NULL, &ol);
res = WriteFile(dev->device_handle, buf, (DWORD)length, NULL, &ol);

if (!res) {
if (GetLastError() != ERROR_IO_PENDING) {
Expand Down Expand Up @@ -678,7 +678,7 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char
dev->read_pending = TRUE;
memset(dev->read_buf, 0, dev->input_report_length);
ResetEvent(ev);
res = ReadFile(dev->device_handle, dev->read_buf, dev->input_report_length, &bytes_read, &dev->ol);
res = ReadFile(dev->device_handle, dev->read_buf, (DWORD)dev->input_report_length, &bytes_read, &dev->ol);

if (!res) {
if (GetLastError() != ERROR_IO_PENDING) {
Expand Down Expand Up @@ -732,7 +732,7 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char
return -1;
}

return copy_len;
return (int)copy_len;
}

int HID_API_EXPORT HID_API_CALL hid_read(hid_device *dev, unsigned char *data, size_t length)
Expand All @@ -748,13 +748,13 @@ int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device *dev, int nonbloc

int HID_API_EXPORT HID_API_CALL hid_send_feature_report(hid_device *dev, const unsigned char *data, size_t length)
{
BOOL res = HidD_SetFeature(dev->device_handle, (PVOID)data, length);
BOOL res = HidD_SetFeature(dev->device_handle, (PVOID)data, (ULONG)length);
if (!res) {
register_error(dev, "HidD_SetFeature");
return -1;
}

return length;
return (int)length;
}


Expand All @@ -776,8 +776,8 @@ int HID_API_EXPORT HID_API_CALL hid_get_feature_report(hid_device *dev, unsigned

res = DeviceIoControl(dev->device_handle,
IOCTL_HID_GET_FEATURE,
data, length,
data, length,
data, (DWORD)length,
data, (DWORD)length,
&bytes_returned, &ol);

if (!res) {
Expand Down Expand Up @@ -818,7 +818,7 @@ int HID_API_EXPORT_CALL HID_API_CALL hid_get_manufacturer_string(hid_device *dev
{
BOOL res;

res = HidD_GetManufacturerString(dev->device_handle, string, sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS));
res = HidD_GetManufacturerString(dev->device_handle, string, (ULONG)(sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS)));
if (!res) {
register_error(dev, "HidD_GetManufacturerString");
return -1;
Expand All @@ -831,7 +831,7 @@ int HID_API_EXPORT_CALL HID_API_CALL hid_get_product_string(hid_device *dev, wch
{
BOOL res;

res = HidD_GetProductString(dev->device_handle, string, sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS));
res = HidD_GetProductString(dev->device_handle, string, (ULONG)(sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS)));
if (!res) {
register_error(dev, "HidD_GetProductString");
return -1;
Expand All @@ -844,7 +844,7 @@ int HID_API_EXPORT_CALL HID_API_CALL hid_get_serial_number_string(hid_device *de
{
BOOL res;

res = HidD_GetSerialNumberString(dev->device_handle, string, sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS));
res = HidD_GetSerialNumberString(dev->device_handle, string, (ULONG)(sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS)));
if (!res) {
register_error(dev, "HidD_GetSerialNumberString");
return -1;
Expand All @@ -857,7 +857,7 @@ int HID_API_EXPORT_CALL HID_API_CALL hid_get_indexed_string(hid_device *dev, int
{
BOOL res;

res = HidD_GetIndexedString(dev->device_handle, string_index, string, sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS));
res = HidD_GetIndexedString(dev->device_handle, string_index, string, (ULONG)(sizeof(wchar_t) * MIN(maxlen, MAX_STRING_WCHARS)));
if (!res) {
register_error(dev, "HidD_GetIndexedString");
return -1;
Expand Down