Hi Alex,
Just a few things I noticed.
On Monday, 17 April 2017 2:23 PM, Alex Henrie wrote:
+ /* create registry entry */ + sprintfW( reg_value, reg_value_format, n ); + RegSetValueExW( windows_ports_key, nt_name.Buffer, 0, REG_SZ, + (BYTE *)reg_value, strlenW( reg_value ) * sizeof(WCHAR) );
+ return TRUE; +}
You need to pass RegSetValueExW() the string length plus the null-terminating character.
+/* find and create serial or parallel ports */ +static void create_port_devices( DRIVER_OBJECT *driver ) +{ + static const char *serial_search_paths[] = { +#ifdef linux + "/dev/ttyS%u", + "/dev/ttyUSB%u", +#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) + "/dev/cuau%u", +#elif defined(__DragonFly__) + "/dev/cuaa%u", +#else + "", +#endif + }; + static const WCHAR serialcomm_keyW[] = {'H','A','R','D','W','A','R','E','\', + 'D','E','V','I','C','E','M','A','P','\', + 'S','E','R','I','A','L','C','O','M','M',0}; + const char **search_paths; + const WCHAR *windows_ports_key_name; + char *dosdevices_path, *p; + HKEY wine_ports_key = NULL, windows_ports_key = NULL; + char unix_path[256]; + int i, j, n = 1;
+ if (!(dosdevices_path = get_dosdevices_path( &p ))) + return;
+ if (driver == serial_driver) + { + p[0] = 'c'; + p[1] = 'o'; + p[2] = 'm'; + search_paths = serial_search_paths; + windows_ports_key_name = serialcomm_keyW; + } + else + { + /* TODO: support parallel ports */ + } + p += 3;
+ RegOpenKeyW( HKEY_LOCAL_MACHINE, ports_keyW, &wine_ports_key ); + RegOpenKeyW( HKEY_LOCAL_MACHINE, windows_ports_key_name, &windows_ports_key );
Please use RegOpenKeyExW() and check for any failures from these functions.
+ /* remove old symlinks */ + for (n = 1; n <= MAX_PORTS; n++) + { + sprintf( p, "%u", n ); + if (unlink( dosdevices_path ) != 0 && !errno) + break; + }
+ /* look for ports in the usual places */ + n = 1; + for (i = 0; i < sizeof(search_paths)/sizeof(search_paths[0]); i++)
This won't do what you expect. sizeof(search_paths)/sizeof(search_paths[0]) is 1. You need to pass in the number of elements in the array. Also, it's better to avoid making this calculation during each loop.
Hugh
2017-04-18 7:00 GMT-06:00 Hugh McMaster hugh.mcmaster@outlook.com:
Hi Alex,
Just a few things I noticed.
On Monday, 17 April 2017 2:23 PM, Alex Henrie wrote:
- /* create registry entry */
- sprintfW( reg_value, reg_value_format, n );
- RegSetValueExW( windows_ports_key, nt_name.Buffer, 0, REG_SZ,
(BYTE *)reg_value, strlenW( reg_value ) * sizeof(WCHAR) );
- return TRUE;
+}
You need to pass RegSetValueExW() the string length plus the null-terminating character.
Good catch.
+/* find and create serial or parallel ports */ +static void create_port_devices( DRIVER_OBJECT *driver ) +{
- static const char *serial_search_paths[] = {
+#ifdef linux
"/dev/ttyS%u",
"/dev/ttyUSB%u",
+#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
"/dev/cuau%u",
+#elif defined(__DragonFly__)
"/dev/cuaa%u",
+#else
"",
+#endif
- };
- static const WCHAR serialcomm_keyW[] = {'H','A','R','D','W','A','R','E','\',
'D','E','V','I','C','E','M','A','P','\\',
'S','E','R','I','A','L','C','O','M','M',0};
- const char **search_paths;
- const WCHAR *windows_ports_key_name;
- char *dosdevices_path, *p;
- HKEY wine_ports_key = NULL, windows_ports_key = NULL;
- char unix_path[256];
- int i, j, n = 1;
- if (!(dosdevices_path = get_dosdevices_path( &p )))
return;
- if (driver == serial_driver)
- {
p[0] = 'c';
p[1] = 'o';
p[2] = 'm';
search_paths = serial_search_paths;
windows_ports_key_name = serialcomm_keyW;
- }
- else
- {
/* TODO: support parallel ports */
- }
- p += 3;
- RegOpenKeyW( HKEY_LOCAL_MACHINE, ports_keyW, &wine_ports_key );
- RegOpenKeyW( HKEY_LOCAL_MACHINE, windows_ports_key_name, &windows_ports_key );
Please use RegOpenKeyExW() and check for any failures from these functions.
If these functions fail, wine_ports_key and windows_ports_key will still be NULL, and symlinks will be created for the detected devices even though registry entries will not. I think that is desired behavior.
- /* remove old symlinks */
- for (n = 1; n <= MAX_PORTS; n++)
- {
sprintf( p, "%u", n );
if (unlink( dosdevices_path ) != 0 && !errno)
break;
- }
- /* look for ports in the usual places */
- n = 1;
- for (i = 0; i < sizeof(search_paths)/sizeof(search_paths[0]); i++)
This won't do what you expect. sizeof(search_paths)/sizeof(search_paths[0]) is 1. You need to pass in the number of elements in the array. Also, it's better to avoid making this calculation during each loop.
Good catch.
Thanks for the feedback, that was very helpful!
-Alex