[PATCH 1/2] mountmgr: Create devices and registry entries for serial ports.
Alex Henrie
alexhenrie24 at gmail.com
Tue Apr 18 09:54:17 CDT 2017
2017-04-18 7:00 GMT-06:00 Hugh McMaster <hugh.mcmaster at 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
More information about the wine-devel
mailing list