[1/3] scrrun: Implement filesys_DriveExists.
joachim.priesner at web.de
Thu Oct 15 12:39:38 CDT 2015
Thanks for having a look at the code, will fix the method wrapper issue and resend later.
> Not necessary wrong, but it'd be better if all that validation was
> GetDriveTypeW responsibility. I don't see many tests for it in
> kernel32/tests, could you please add some, so we can get rid of this
> extra logic?
GetDriveTypeW does not accept strings consisting of only a single drive letter, but filesys_DriveExists does. So we cannot rely on GetDriveTypeW's validation and have to do our own.
I'll send the GetDriveType tests in a separate patch, good to have them anyway :)
> I haven't looked much at its implementation but it feels
> like DRIVE_UNKNOWN would also qualify as non-existent drive.
The Windows implementation of DriveExists returns true for DRIVE_UNKNOWN. (I tested using a native scrrun.dll by patching GetDriveTypeW to always return DRIVE_UNKNOWN.)
> > + DriveSpec = toupperW(DriveSpec);
> > + if (DriveSpec < 'A' || DriveSpec > 'Z'
> > + || (len >= 2 && DriveSpec != ':')
> > + || (len == 3 && DriveSpec != '\\'))
> > + return E_INVALIDARG;
> > + hr = filesys_DriveExists(iface, DriveSpec, &drive_exists);
> I'm not sure about that sanity check, the reason is that DriveExists
> doesn't fail on invalid drivespec?
Correct, we cannot distinguish between a nonexisting drive and a malformed parameter just by looking at hr.
More information about the wine-devel