Re: [ros-dev] [ros-diffs] [janderwald] 40694: - Fix allocation of CM_RESOURCE_LIST - Might fix bug 4354 See issue #4354 for more details.

[Timo Kreuzer]

Isn't that actually the same?

It also ignores the number of full descriptors, which I guess is wrong.
And it looks like the whole parsing code is broken. It ignores the
number of partial descriptors and does FullList++

Timo

[e-mail] schrieb:
> Author: janderwald
> Date: Sat Apr 25 16:05:08 2009
> New Revision: 40694
>
> URL: http://svn.reactos.org/svn/reactos?rev=40694&view=rev
> Log:
> Fix allocation of CM_RESOURCE_LIST
> Might fix bug 4354
>
> See issue #4354 for more details.
>
> Modified:
>     trunk/reactos/drivers/video/videoprt/dispatch.c
>
> Modified: trunk/reactos/drivers/video/videoprt/dispatch.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/video/videoprt/disp...
> ==============================================================================
> --- trunk/reactos/drivers/video/videoprt/dispatch.c [iso-8859-1] (original)
> +++ trunk/reactos/drivers/video/videoprt/dispatch.c [iso-8859-1] Sat Apr 25 16:05:08 2009
> @@ -340,9 +340,11 @@
>    /* Save the resource list */
>    ResourceCount = AllocatedResources->List[0].PartialResourceList.Count;
>  ResourceListSize =
>     FIELD_OFFSET(CM_RESOURCE_LIST, List[0].PartialResourceList.
>                  PartialDescriptors[ResourceCount]);
>  ResourceListSize = sizeof(CM_RESOURCE_LIST);
>  if (ResourceCount > 1)
>     ResourceListSize += (ResourceCount-1) * sizeof(CM_PARTIAL_RESOURCE_DESCRIPTOR);
>  DeviceExtension->AllocatedResources = ExAllocatePool(PagedPool, ResourceListSize);
>  if (DeviceExtension->AllocatedResources == NULL)
>  {

[Johannes Anderwald]

Timo Kreuzer wrote:
> Isn't that actually the same?

Hi,

IMO the old code only calculated the offset from PartialDescriptors 
array to CM_RESOURCE_LIST. This works fine if there are no partial 
descriptors. But it breaks if there are.
> It also ignores the number of full descriptors, which I guess is wrong.
> And it looks like the whole parsing code is broken. It ignores the
> number of partial descriptors and does FullList++

The MSDN states the number of CM_FULL_RESOURCE_DESCRIPTOR in 
CM_RESOURCE_LIST is always 1 for WDM drivers.
The partial descriptors are handeled in the 2nd for loop.
> Timo

regards,
Johannes

[Timo Kreuzer]

Johannes Anderwald schrieb:
> Timo Kreuzer wrote:
> > Isn't that actually the same?
>
> Hi,
>
> IMO the old code only calculated the offset from PartialDescriptors 
> array to CM_RESOURCE_LIST. This works fine if there are no partial 
> descriptors. But it breaks if there are.

It was

FIELD_OFFSET(CM_RESOURCE_LIST, List[0].PartialResourceList.PartialDescriptors[ResourceCount]);

So it should have calculated the offset of
PartialDescriptors[ResourceCount] which is one after the last one that
belongs to the list or (ResourceCount-1) entries after the end of
CM_RESOURCE_LIST.
> > It also ignores the number of full descriptors, which I guess is wrong.
> > And it looks like the whole parsing code is broken. It ignores the
> > number of partial descriptors and does FullList++
>
> The MSDN states the number of CM_FULL_RESOURCE_DESCRIPTOR in 
> CM_RESOURCE_LIST is always 1 for WDM drivers.
> The partial descriptors are handeled in the 2nd for loop.

Ah, ok, but in this case the outer loop makes no sense and is only
confusing. Maybe an ASSERT(..Count == 1) should be put instead?