Am Donnerstag, 4. August 2005 17:40 schrieb Dave Jones:
>On Thu, Aug 04, 2005 at 11:38:30AM +0200, Rolf Eike Beer wrote:
> > >+ ulFibreFrame = kmalloc((2048/4), GFP_KERNEL);
> >
> > The size bug was already found by Dave Jones. This never should be
> > written this way (not your fault). The array should have been
> > [2048/sizeof(ULONG)].
>
>wasteful. We only ever use 2048 bytes of this array, so doubling
>its size on 64bit is pointless, unless you make changes later on
>in the driver. (Which I think don't make sense, as we just copy
>32 64byte chunks).
No, this is how it should have been before. This way it would have been clear
where the magic 4 came from.
>Ermm, actually this looks totally bogus..
>CpqTsGetSFQEntry() ...
>
> if( total_bytes <= 2048 )
> {
> memcpy( ulDestPtr,
> &fcChip->SFQ->QEntry[consumerIndex],
> 64 ); // each SFQ entry is 64 bytes
> ulDestPtr += 16; // advance pointer to next 64 byte block
> }
>
>we're trashing the last 48 bytes of every copy we make.
>Does this driver even work ?
No, ulDestPtr ist ULONG* so we increase it by sizeof(ULONG)*16 which is 64.
This is one of the places I was talking about where people might miss what's
going on. ;) IMHO it makes absolutely no sense to use a ULONG* at this place.
Eike
Attachment:
pgp5KMktgVrY6.pgp
Description: PGP signature
- Follow-Ups:
- References:
- [PATCH 1/2] cpqfc: fix for "Using too much stach" in 2.6 kernel
- From: "Saripalli, Venkata Ramanamurthy (STSD)" <[email protected]>
- Re: [PATCH 1/2] cpqfc: fix for "Using too much stach" in 2.6 kernel
- From: Rolf Eike Beer <[email protected]>
- Re: [PATCH 1/2] cpqfc: fix for "Using too much stach" in 2.6 kernel
- From: Dave Jones <[email protected]>
- [PATCH 1/2] cpqfc: fix for "Using too much stach" in 2.6 kernel
- Prev by Date: Re: [PATCH] 3/5 explicit-iopl
- Next by Date: Re: [patch 2.6.13-rc4] fix get_user_pages bug
- Previous by thread: Re: [PATCH 1/2] cpqfc: fix for "Using too much stach" in 2.6 kernel
- Next by thread: Re: [PATCH 1/2] cpqfc: fix for "Using too much stach" in 2.6 kernel
- Index(es):
