Hi Corinna,

Corinna Vinschen wrote:
> Hi Christian,
>
> patch looks pretty good to me.  Just two minor points:
>
>> +  /* Traverse \Device directory ... */
>> +  bool errno_set = false;
>> +  HANDLE devhdl = INVALID_HANDLE_VALUE;
> INVALID_HANDLE_VALUE is a Win32 API concept and is only returned by
> CreateFile and friends.  The native NT API doesn't know
> INVALID_HANDLE_VALUE and, fortunately, only uses NULL.  I think it's
> puzzeling to use INVALID_HANDLE_VALUE in a native NT context. So, would
> you mind to just use NULL (or nullptr) instead?  As in...
>
>> +      if (devhdl != INVALID_HANDLE_VALUE)
>           if (devhdl)
>
> Sounds easier to me :)

Yes...done.


>> +	  /* Fetch drive layout info to get size of all partitions on disk. */
>> +	  DWORD part_cnt = 0;
>> +	  PARTITION_INFORMATION_EX *pix = nullptr;
>> +	  PARTITION_INFORMATION *pi = nullptr;
>> +	  DWORD bytes_read;
>> +	  if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, nullptr, 0,
>> +			       ioctl_buf, NT_MAX_PATH, &bytes_read, nullptr))
>> +	    {
>> +	      DRIVE_LAYOUT_INFORMATION_EX *pdlix =
>> +		  reinterpret_cast<DRIVE_LAYOUT_INFORMATION_EX *>(ioctl_buf);
>> +	      part_cnt = pdlix->PartitionCount;
>> +	      pix = pdlix->PartitionEntry;
>> +	    }
>> +	  else if (DeviceIoControl (devhdl, IOCTL_DISK_GET_DRIVE_LAYOUT, nullptr,
> Do we really still need IOCTL_DISK_GET_DRIVE_LAYOUT w/o _EX?

Possibly not. I borrowed this code from code behind /proc/partitions.


> fhandler_proc still uses it, too, but the EX variation is available
> since XP. I can't imagine that a fallback to the non-EX-version is still
> necessary.  If you like you could drop it immediately, or we can drop
> both usages as a followup patch.

Old IOCTL dropped and code simplified.


> Last, but not least, do you see a chance to add any other /dev/disk
> subdir?  by-partuuid, perhaps?

Possibly, but not very soon. I'm not yet sure which API functions could 
be used.
Some early draft ideas:

/dev/disk/by-partuid (Partition UUID -> device)
   GPT_PART_UUID -> ../../sdXN (GPT partition UUID)
   MBR_SERIAL-partN -> ../../sdYM (Fake UUID for MBR)

/dev/disk/by-uuid (Windows Volume UUID -> device)
   Vol_UUID1 -> ../../sdXN  (disk volume)
   Vol_UUID2 -> ../../scd0  (CD/DVD drive volume)
   Vol_UUID3 -> /proc/sys/GLOBAL??/Volume{UUID}  (others, e.g. VeraCrypt 
volume)

/dev/disk/by-drive (Cygwin specific: drive letter -> volume)
   c -> ../by-uuid/UUID (if UUID available)
   x -> /proc/sys/DosDevices/X: (others, e.g. Network, "mounted" Volume 
Shadow Copy)

Christian