[PATCH] setupapi: Rewrite DiskSpaceList logic using lists.

[Vijay Kiran Kamuju]

On Mon, Apr 29, 2019 at 2:50 AM Jefferson Carpenter
<jeffersoncarpenter2 at gmail.com> wrote:
>
> -typedef struct {
> -    WCHAR   lpzName[20];
> -    LONGLONG dwFreeSpace;
> -    LONGLONG dwWantedSpace;
> -} DRIVE_ENTRY, *LPDRIVE_ENTRY;
> +struct file_entry
> +{
> +    struct list entry;
> +    WCHAR *path;
> +    UINT operation;
> +    LONGLONG size;
> +};
> +
> +struct space_list
> +{
> +    struct list files;
> +    UINT flags;
> +};
>
> -typedef struct {
> -    DWORD   dwDriveCount;
> -    DRIVE_ENTRY Drives[26];
> -} DISKSPACELIST, *LPDISKSPACELIST;
>
> Until tests are written that constrain the behavior of this module, I'm
> not sure it makes sense to alter the structs here.
>
>
> -    rc = GetLogicalDriveStringsW(255,drives);
> -
> -    if (rc == 0)
> -        return NULL;
> -
> -    list = HeapAlloc(GetProcessHeap(),0,sizeof(DISKSPACELIST));
> -
> -    list->dwDriveCount = 0;
> -
> -    ptr = drives;
> -
> -    while (*ptr)
> +    list = HeapAlloc(GetProcessHeap(), 0, sizeof(*list));
> +    if (list)
>       {
> -        DWORD type = GetDriveTypeW(ptr);
> -        if (type == DRIVE_FIXED)
> -        {
> -            DWORD clusters;
> -            DWORD sectors;
> -            DWORD bytes;
> -            DWORD total;
> -            lstrcpyW(list->Drives[list->dwDriveCount].lpzName,ptr);
> -            GetDiskFreeSpaceW(ptr,&sectors,&bytes,&clusters,&total);
> -            list->Drives[list->dwDriveCount].dwFreeSpace = clusters *
> sectors *
> -                                                           bytes;
> -            list->Drives[list->dwDriveCount].dwWantedSpace = 0;
> -            list->dwDriveCount++;
> -        }
> -       ptr += lstrlenW(ptr) + 1;
> +        list->flags = flags;
> +        list_init(&list->files);
>       }
> +
>
> Likewise, I see no reason to alter this behavior until there are tests
> for it.
>
>
> +struct file_entry
> +{
> +    struct list entry;
> +    WCHAR *path;
> +    UINT operation;
> +    LONGLONG size;
> +};
> +
> +struct space_list
> +{
> +    struct list files;
> +    UINT flags;
> +};
>
> If flags and operation were a bitfield and enum respectively (instead of
> UINT and UINT) the compiler could aid us in detecting unintended
> implicit conversions so far as they go, and it would be easier to find
> the values that those fields can correctly be assigned to in the source
> code.  Only at API boundaries must the types be precisely the types
> chosen by Microsoft.
>
>
> -BOOL WINAPI SetupDestroyDiskSpaceList(HDSKSPC DiskSpace)
> +BOOL WINAPI SetupDestroyDiskSpaceList(HDSKSPC diskspace)
>   {
> -    LPDISKSPACELIST list = DiskSpace;
> -    HeapFree(GetProcessHeap(),0,list);
> +    struct space_list *list = diskspace;
> +    struct file_entry *file, *file2;
> +
> +    if (!diskspace)
> +    {
> +        SetLastError(ERROR_INVALID_PARAMETER);
> +        return FALSE;
> +    }
> +
> +    LIST_FOR_EACH_ENTRY_SAFE(file, file2, &list->files, struct
> file_entry, entry)
> +    {
> +        HeapFree(GetProcessHeap(), 0, file->path);
> +        list_remove(&file->entry);
> +        HeapFree(GetProcessHeap(), 0, file);
> +    }
> +
> +    HeapFree(GetProcessHeap(), 0, list);
>       return TRUE;
>   }
>
> Note this function is also longer after the above modifications to the
> data structures.  You should write tests that confirm the computations
> and effects of SetupAPI, and then write the Wine implementation guided
> by those.
>
>
> - Jefferson
This reimplementation is needed for implementation of
SetupAddToDiskSpaceList and related functions.