v2: is this a bug ? (in device_util.c)

Hardwaremain()-> dev_configure();-> void
root_dev_read_resources(device_t root) ->

void compact_resources(device_t dev)->

memmove(resource, resource + 1, dev->resources - i);

The third parameter should be (dev->resources-I)* sizeof(*resource)

[ron minnich]

On Mon, Aug 11, 2008 at 7:30 PM,   wrote:
> Hardwaremain()-> dev_configure();-> void root_dev_read_resources(device_t
> root) ->
>
> void compact_resources(device_t dev)->
>
> memmove(resource, resource + 1, dev->resources - i);
>
> The third parameter should be (dev->resources-I)* sizeof(*resource)

that's a puzzler. I think you are right but ... how did this EVER
work. Only if this code was
never really exercised.

same bug is in v3, btw.

Wow. I am impressed with this catch!

ron

[Stefan Reinauer]

[e-mail] wrote:
> Hardwaremain()-> dev_configure();-> void
> root_dev_read_resources(device_t root) ->
>
> void compact_resources(device_t dev)->
>
> memmove(resource, resource + 1, dev->resources - i);
>
> The third parameter should be (dev->resources-I)* sizeof(*resource)

Dear Jason,

thank you very much for your finding!

It is indeed a bug and should be fixed.

I am surprised we did not hit this before, as this potentially wipes out
whole resource entries. (unless the number of resources is bigger than
the size of a resource in bytes)

But only if there are holes in the resource entries. Not sure why/how
these occur. Eric, your word!

Stefan Reinauer  writes:
> [e-mail] wrote:
> > Hardwaremain()-> dev_configure();-> void
> > root_dev_read_resources(device_t root) ->
> >
> > void compact_resources(device_t dev)->
> >
> > memmove(resource, resource + 1, dev->resources - i);
> >
> > The third parameter should be (dev->resources-I)* sizeof(*resource)
>
> Dear Jason,
>
> thank you very much for your finding!
>
> It is indeed a bug and should be fixed.
>
> I am surprised we did not hit this before, as this potentially wipes out
> whole resource entries. (unless the number of resources is bigger than
> the size of a resource in bytes)
>
> But only if there are holes in the resource entries. Not sure why/how
> these occur. Eric, your word!

Looking at the code I'm fairly certain that we never or practically never
exercise it.  To create a hole it would require someone to call new_resource
and then never setup the resource.  compact_resources looks to be there just
in case that happens, rather than to handle an actual case that someone has
hit.

We have a small static pool of 24 resources on each device, and a struct
resource looks to be 64 bytes.

Eric