[ros-diffs] [ion] 14643: IoCreateDevice Changes:

IoCreateDevice Changes:

    - IoAllocateVpb renamed to IopAllocateVpb and made cleaner
    - Let ObCreateObject do all the memory allocation since the extra
data belongs to the
      object and it should be responsible for allocating it. No more
extra pool allocations
      or deallocations.
    - Zero out everythign a single time.
    - Remove useless code duplication
    - Use proper I/O Manager types for the object headers.
    - Honour Exclusive Flag, and Has Name Flag.
    - Only initialize event if there is a volume.
    - Initialize a VPB for Virtual Disks also.
    - Set up Device Object Extension properly
    - Set DO_DEVICE_INITIALIZING flag.
    - Use proper sector sizes, don't hardcode 512 (should be 2048 for
cds)
    - Actually insert the object into the object table with
ObInsertObject. This might seem
      useless, but remember that's simply because ROS has a broken Ob
Manager which does way
      too much in ObCreateObject. It will be easier to use the rewrite
if this is done properly.
    - Set the right sizes in some places.
Modified: trunk/reactos/include/ddk/iotypes.h
Modified: trunk/reactos/ntoskrnl/include/internal/io.h
Modified: trunk/reactos/ntoskrnl/io/device.c
Modified: trunk/reactos/ntoskrnl/io/vpb.c
  _____  

Modified: trunk/reactos/include/ddk/iotypes.h
--- trunk/reactos/include/ddk/iotypes.h	2005-04-16 22:11:59 UTC (rev
14642)
+++ trunk/reactos/include/ddk/iotypes.h	2005-04-17 04:20:16 UTC (rev
14643)
@@ -892,8 +892,19 @@

 } FAST_IO_DISPATCH_TABLE, * PFAST_IO_DISPATCH_TABLE;
 #endif
 
-#define IO_TYPE_DRIVER 4L
-#define IO_TYPE_FILE 0x0F5L
+#define IO_TYPE_ADAPTER                 0x1L
+#define IO_TYPE_CONTROLLER              0x2L
+#define IO_TYPE_DEVICE                  0x3L
+#define IO_TYPE_DRIVER                  0x4L
+#define IO_TYPE_FILE                    0x0F5L /* Temp Hack */
+#define IO_TYPE_IRP                     0x6L
+#define IO_TYPE_MASTER_ADAPTER          0x7L
+#define IO_TYPE_OPEN_PACKET             0x8L
+#define IO_TYPE_TIMER                   0x9L
+#define IO_TYPE_VPB                     0xaL
+#define IO_TYPE_ERROR_LOG               0xbL
+#define IO_TYPE_ERROR_MESSAGE           0xcL
+#define IO_TYPE_DEVICE_OBJECT_EXTENSION 0xdL
 
 #define DRVO_UNLOAD_INVOKED 0x1L
 #define DRVO_LEGACY_DRIVER  0x2L
@@ -959,7 +970,6 @@
 /*
  * PURPOSE: Special timer associated with each device
  */
- #define IO_TYPE_TIMER 9
 typedef struct _IO_TIMER {
    USHORT Type;				/* Every IO Object has a
Type */
    USHORT TimerEnabled;			/* Tells us if the Timer
is enabled or not */
  _____  

Modified: trunk/reactos/ntoskrnl/include/internal/io.h
--- trunk/reactos/ntoskrnl/include/internal/io.h	2005-04-16
22:11:59 UTC (rev 14642)
+++ trunk/reactos/ntoskrnl/include/internal/io.h	2005-04-17
04:20:16 UTC (rev 14643)
@@ -370,8 +370,11 @@

 		PVOID Parent,
 		PWSTR RemainingPath,
 		POBJECT_ATTRIBUTES ObjectAttributes);
-NTSTATUS IoAttachVpb(PDEVICE_OBJECT DeviceObject);
 
+NTSTATUS 
+STDCALL
+IopAttachVpb(PDEVICE_OBJECT DeviceObject);
+
 PIRP IoBuildSynchronousFsdRequestWithMdl(ULONG MajorFunction,
 					 PDEVICE_OBJECT DeviceObject,
 					 PMDL Mdl,
  _____  

Modified: trunk/reactos/ntoskrnl/io/device.c
--- trunk/reactos/ntoskrnl/io/device.c	2005-04-16 22:11:59 UTC (rev
14642)
+++ trunk/reactos/ntoskrnl/io/device.c	2005-04-17 04:20:16 UTC (rev
14643)
@@ -307,7 +307,6 @@

     SourceDeviceExtension = SourceDevice->DeviceObjectExtension;
     
     /* Make sure that it's in a correct state */
-    DPRINT1("flags %d\n",
AttachedDevice->DeviceObjectExtension->ExtensionFlags);
     if (!(AttachedDevice->DeviceObjectExtension->ExtensionFlags & 
         (DOE_UNLOAD_PENDING | DOE_DELETE_PENDING | 
          DOE_REMOVE_PENDING | DOE_REMOVE_PROCESSED)))
@@ -355,10 +354,6 @@
       ExFreePool(DeviceObject->Timer);
    }
 
-   /* Free device extension */
-   if (DeviceObject->DeviceObjectExtension)
-      ExFreePool(DeviceObject->DeviceObjectExtension);
-
    /* Remove device from driver device list */
    Previous = DeviceObject->DriverObject->DeviceObject;
    if (Previous == DeviceObject)
@@ -652,157 +647,169 @@
  * Status
  *    @implemented
  */
-NTSTATUS STDCALL
-IoCreateDevice(
-   PDRIVER_OBJECT DriverObject,
-   ULONG DeviceExtensionSize,
-   PUNICODE_STRING DeviceName,
-   DEVICE_TYPE DeviceType,
-   ULONG DeviceCharacteristics,
-   BOOLEAN Exclusive,
-   PDEVICE_OBJECT *DeviceObject)
+NTSTATUS 
+STDCALL
+IoCreateDevice(PDRIVER_OBJECT DriverObject,
+               ULONG DeviceExtensionSize,
+               PUNICODE_STRING DeviceName,
+               DEVICE_TYPE DeviceType,
+               ULONG DeviceCharacteristics,
+               BOOLEAN Exclusive,
+               PDEVICE_OBJECT *DeviceObject)
 {
-   WCHAR AutoNameBuffer[20];
-   UNICODE_STRING AutoName;
-   PDEVICE_OBJECT CreatedDeviceObject;
-   PDEVOBJ_EXTENSION DeviceObjectExtension;
-   OBJECT_ATTRIBUTES ObjectAttributes;
-   NTSTATUS Status;
+    WCHAR AutoNameBuffer[20];
+    UNICODE_STRING AutoName;
+    PDEVICE_OBJECT CreatedDeviceObject;
+    PDEVOBJ_EXTENSION DeviceObjectExtension;
+    OBJECT_ATTRIBUTES ObjectAttributes;
+    NTSTATUS Status;
+    ULONG AlignedDeviceExtensionSize;
+    ULONG TotalSize;
+    HANDLE TempHandle;
    
-   ASSERT_IRQL(PASSIVE_LEVEL);
+    ASSERT_IRQL(PASSIVE_LEVEL);
+    DPRINT("IoCreateDevice(DriverObject %x)\n",DriverObject);
    
-   if (DeviceName != NULL)
-   {
-      DPRINT("IoCreateDevice(DriverObject %x, DeviceName %S)\n",
-         DriverObject, DeviceName->Buffer);
+    /* Generate a name if we have to */
+    if (DeviceCharacteristics & FILE_AUTOGENERATED_DEVICE_NAME)
+    {
+        swprintf(AutoNameBuffer,
+                 L"\\Device\\%08lx",
+                 InterlockedIncrementUL(&IopDeviceObjectNumber));
+        RtlInitUnicodeString(&AutoName, AutoNameBuffer);
+        DeviceName = &AutoName;
    }
-   else
-   {
-      DPRINT("IoCreateDevice(DriverObject %x)\n",DriverObject);
-   }
    
-   if (DeviceCharacteristics & FILE_AUTOGENERATED_DEVICE_NAME)
-   {
-      swprintf(AutoNameBuffer,
-               L"\\Device\\%08lx",
-               InterlockedIncrementUL(&IopDeviceObjectNumber));
-      RtlInitUnicodeString(&AutoName,
-                           AutoNameBuffer);
-      DeviceName = &AutoName;
-   }
-   
-   if (DeviceName != NULL)
-   {
-      InitializeObjectAttributes(&ObjectAttributes, DeviceName, 0,
NULL, NULL);
-      Status = ObCreateObject(
-         KernelMode,
-         IoDeviceObjectType,
-         &ObjectAttributes,
-         KernelMode,
-         NULL,
-         sizeof(DEVICE_OBJECT),
-         0,
-         0,
-         (PVOID*)&CreatedDeviceObject);
-   }
-   else
-   {
-      Status = ObCreateObject(
-         KernelMode,
-         IoDeviceObjectType,
-         NULL,
-         KernelMode,
-         NULL,
-         sizeof(DEVICE_OBJECT),
-         0,
-         0,
-         (PVOID*)&CreatedDeviceObject);
-   }
-   
-   *DeviceObject = NULL;
-   
-   if (!NT_SUCCESS(Status))
-   {
-      DPRINT("IoCreateDevice() ObCreateObject failed, status:
0x%08X\n", Status);
-      return Status;
-   }
-  
-   if (DriverObject->DeviceObject == NULL)
-   {
-      DriverObject->DeviceObject = CreatedDeviceObject;
-      CreatedDeviceObject->NextDevice = NULL;
-   }
-   else
-   {
-      CreatedDeviceObject->NextDevice = DriverObject->DeviceObject;
-      DriverObject->DeviceObject = CreatedDeviceObject;
-   }
-  
-   CreatedDeviceObject->Type = DeviceType;
-   CreatedDeviceObject->DriverObject = DriverObject;
-   CreatedDeviceObject->CurrentIrp = NULL;
-   CreatedDeviceObject->Flags = 0;
+    /* Initialize the Object Attributes */
+    InitializeObjectAttributes(&ObjectAttributes, DeviceName, 0, NULL,
NULL);
+    
+    /* Honour exclusive flag */
+    ObjectAttributes.Attributes |= OBJ_EXCLUSIVE;
+    
+    /* Align the Extension Size to 8-bytes */
+    AlignedDeviceExtensionSize = (DeviceExtensionSize + 7) &~ 7;
+    DPRINT("AlignedDeviceExtensionSize %x\n",
AlignedDeviceExtensionSize);
+    
+    /* Total Size */
+    TotalSize = AlignedDeviceExtensionSize + 
+                sizeof(DEVICE_OBJECT) + sizeof(DEVOBJ_EXTENSION);
+    DPRINT("TotalSize %x\n", TotalSize);
 
-   CreatedDeviceObject->DeviceExtension = 
-      ExAllocatePoolWithTag(
-         NonPagedPool,
-         DeviceExtensionSize,
-         TAG_DEVICE_EXTENSION);
-
-   if (DeviceExtensionSize > 0 && CreatedDeviceObject->DeviceExtension
== NULL)
-   {
-      ExFreePool(CreatedDeviceObject);
-      DPRINT("IoCreateDevice() ExAllocatePoolWithTag failed, returning:
0x%08X\n", STATUS_INSUFFICIENT_RESOURCES);
-      return STATUS_INSUFFICIENT_RESOURCES;
-   }
-
-   if (DeviceExtensionSize > 0)
-   {
-      RtlZeroMemory(CreatedDeviceObject->DeviceExtension,
DeviceExtensionSize);
-   }
-
-   CreatedDeviceObject->Size = sizeof(DEVICE_OBJECT) +
DeviceExtensionSize;
-   CreatedDeviceObject->ReferenceCount = 1;
-   CreatedDeviceObject->AttachedDevice = NULL;
-   CreatedDeviceObject->DeviceType = DeviceType;
-   CreatedDeviceObject->StackSize = 1;
-   CreatedDeviceObject->AlignmentRequirement = 1;
-   CreatedDeviceObject->Characteristics = DeviceCharacteristics;
-   CreatedDeviceObject->Timer = NULL;
-   CreatedDeviceObject->Vpb = NULL;
-   KeInitializeDeviceQueue(&CreatedDeviceObject->DeviceQueue);
+    /* Create the Device Object */
+    Status = ObCreateObject(KernelMode,
+                            IoDeviceObjectType,
+                            &ObjectAttributes,
+                            KernelMode,
+                            NULL,
+                            TotalSize,
+                            0,
+                            0,
+                            (PVOID*)&CreatedDeviceObject);
   
-   KeInitializeEvent(
-      &CreatedDeviceObject->DeviceLock,
-      SynchronizationEvent,
-      TRUE);
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("IoCreateDevice() ObCreateObject failed, status:
0x%08X\n", Status);
+        return Status;
+    }
+    
+    /* Clear the whole Object and extension so we don't null stuff
manually */
+    RtlZeroMemory(CreatedDeviceObject, TotalSize);
+    DPRINT("CreatedDeviceObject %x\n", CreatedDeviceObject);
   
-   /* FIXME: Do we need to add network drives too?! */
-   if (CreatedDeviceObject->DeviceType == FILE_DEVICE_DISK ||
-       CreatedDeviceObject->DeviceType == FILE_DEVICE_CD_ROM ||
-       CreatedDeviceObject->DeviceType == FILE_DEVICE_TAPE)
-   {
-      IoAttachVpb(CreatedDeviceObject);
-   }
-   CreatedDeviceObject->SectorSize = 512; /* FIXME */
+    /* 
+     * Setup the Type and Size. Note that we don't use the aligned
size,
+     * because that's only padding for the DevObjExt and not part of
the Object.
+     */
+    CreatedDeviceObject->Type = IO_TYPE_DEVICE;
+    CreatedDeviceObject->Size = sizeof(DEVICE_OBJECT) +
DeviceExtensionSize;
+    
+    /* The kernel extension is after the driver internal extension */
+    DeviceObjectExtension = (PDEVOBJ_EXTENSION)
+                            ((ULONG_PTR)(CreatedDeviceObject + 1) + 
+                             AlignedDeviceExtensionSize);
+                     
+    /* Set the Type and Size. Question: why is Size 0 on Windows? */
+    DPRINT("DeviceObjectExtension %x\n", DeviceObjectExtension);
+    DeviceObjectExtension->Type = IO_TYPE_DEVICE_OBJECT_EXTENSION;
+    DeviceObjectExtension->Size = 0;
+                                     
+    /* Link the Object and Extension */
+    DeviceObjectExtension->DeviceObject = CreatedDeviceObject;
+    CreatedDeviceObject->DeviceObjectExtension = DeviceObjectExtension;
+                             
+    /* Set Device Object Data */    
+    CreatedDeviceObject->DeviceType = DeviceType;
+    CreatedDeviceObject->Characteristics = DeviceCharacteristics;
+    CreatedDeviceObject->DeviceExtension = CreatedDeviceObject + 1;
+    CreatedDeviceObject->StackSize = 1;
+    CreatedDeviceObject->AlignmentRequirement = 1; /* FIXME */
+    
+    /* Set the Flags */
+    /* FIXME: After the Driver is Loaded, the flag below should be
removed */
+    CreatedDeviceObject->Flags = DO_DEVICE_INITIALIZING;
+    if (Exclusive) CreatedDeviceObject->Flags |= DO_EXCLUSIVE;
+    if (DeviceName) CreatedDeviceObject->Flags |= DO_DEVICE_HAS_NAME;
   
-   DeviceObjectExtension =
-      ExAllocatePoolWithTag(
-         NonPagedPool,
-         sizeof(DEVOBJ_EXTENSION),
-         TAG_DEVICE_EXTENSION);
+    /* Attach a Vpb for Disks and Tapes, and create the Device Lock */
+    if (CreatedDeviceObject->DeviceType == FILE_DEVICE_DISK ||
+        CreatedDeviceObject->DeviceType == FILE_DEVICE_VIRTUAL_DISK ||
+        CreatedDeviceObject->DeviceType == FILE_DEVICE_CD_ROM ||
+        CreatedDeviceObject->DeviceType == FILE_DEVICE_TAPE)
+    {
+        /* Create Vpb */
+        IopAttachVpb(CreatedDeviceObject);
+        
+        /* Initialize Lock Event */
+        KeInitializeEvent(&CreatedDeviceObject->DeviceLock,
+                          SynchronizationEvent,
+                          TRUE);
+    }
+       
+    /* Set the right Sector Size */
+    switch (DeviceType)
+    {
+        case FILE_DEVICE_DISK_FILE_SYSTEM:
+        case FILE_DEVICE_DISK:
+        case FILE_DEVICE_VIRTUAL_DISK:
+            CreatedDeviceObject->SectorSize  = 512;
+            break;
 
-   RtlZeroMemory(DeviceObjectExtension, sizeof(DEVOBJ_EXTENSION));
-   DeviceObjectExtension->Type = 0 /* ?? */;
-   DeviceObjectExtension->Size = sizeof(DEVOBJ_EXTENSION);
-   DeviceObjectExtension->DeviceObject = CreatedDeviceObject;
-   DeviceObjectExtension->DeviceNode = NULL;
+        case FILE_DEVICE_CD_ROM_FILE_SYSTEM:
+            CreatedDeviceObject->SectorSize = 2048;
+            break;
+    }
+    
+    /* Create the Device Queue */
+    KeInitializeDeviceQueue(&CreatedDeviceObject->DeviceQueue);
 
-   CreatedDeviceObject->DeviceObjectExtension = DeviceObjectExtension;
-
-   *DeviceObject = CreatedDeviceObject;
-  
-   return STATUS_SUCCESS;
+    /* Insert the Object */
+    Status = ObInsertObject(CreatedDeviceObject,
+                            NULL,
+                            FILE_READ_DATA | FILE_WRITE_DATA,
+                            0,
+                            NULL,
+                            &TempHandle);
+                            
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("Cannot insert Device Object into Handle Table\n");
+        *DeviceObject = NULL;
+        return Status;
+    }
+    
+    /* Now do the final linking */
+    ObReferenceObject(DriverObject);
+    CreatedDeviceObject->DriverObject = DriverObject;
+    CreatedDeviceObject->NextDevice = DriverObject->DeviceObject;
+    DriverObject->DeviceObject = CreatedDeviceObject;
+    
+    /* Close the temporary handle, but do an extra reference first so
it doesn't die */
+    ObReferenceObject(CreatedDeviceObject);
+    NtClose(TempHandle);
+    
+    /* Return to caller */
+    *DeviceObject = CreatedDeviceObject;
+    return STATUS_SUCCESS;
 }
 
 /*
  _____  

Modified: trunk/reactos/ntoskrnl/io/vpb.c
--- trunk/reactos/ntoskrnl/io/vpb.c	2005-04-16 22:11:59 UTC (rev
14642)
+++ trunk/reactos/ntoskrnl/io/vpb.c	2005-04-17 04:20:16 UTC (rev
14643)
@@ -30,35 +30,30 @@

 }
 
 NTSTATUS
-IoAttachVpb(PDEVICE_OBJECT DeviceObject)
+STDCALL
+IopAttachVpb(PDEVICE_OBJECT DeviceObject)
 {
-   PVPB Vpb;
+    PVPB Vpb;
    
-   Vpb = ExAllocatePoolWithTag(NonPagedPool,
-			       sizeof(VPB),
-			       TAG_VPB);
-   if (Vpb == NULL)
-     {
-	return(STATUS_UNSUCCESSFUL);
-     }
+    /* Allocate the Vpb */
+    Vpb = ExAllocatePoolWithTag(NonPagedPool,
+                                sizeof(VPB),
+                                TAG_VPB);
+    if (Vpb == NULL) return(STATUS_UNSUCCESSFUL);
+    
+    /* Clear it so we don't waste time manually */
+    RtlZeroMemory(Vpb, sizeof(VPB));
    
-   Vpb->Type = 0;
-   Vpb->Size = sizeof(VPB) / sizeof(DWORD);
-   Vpb->Flags = 0;
-   Vpb->VolumeLabelLength = 0;
-   Vpb->DeviceObject = NULL;
-   Vpb->RealDevice = DeviceObject;
-   Vpb->SerialNumber = 0;
-   Vpb->ReferenceCount = 0;
-   RtlZeroMemory(Vpb->VolumeLabel,
-		 sizeof(WCHAR) * MAXIMUM_VOLUME_LABEL_LENGTH);
-   
-   DeviceObject->Vpb = Vpb;
-   
-   return(STATUS_SUCCESS);
+    /* Set the Header and Device Field */
+    Vpb->Type = IO_TYPE_VPB;
+    Vpb->Size = sizeof(VPB);
+    Vpb->RealDevice = DeviceObject;
+
+    /* link it to the Device Object */
+    DeviceObject->Vpb = Vpb;
+    return(STATUS_SUCCESS);
 }
 
-
 /*
  * FUNCTION: Queries the volume information
  * ARGUMENTS: