On Thu, Nov 03, 2011 at 07:34:54PM +0200, Joe Perches wrote: > On Thu, 2011-11-03 at 12:25 -0300, Diego F. Marfil wrote: > > diff --git a/drivers/staging/bcm/hostmibs.c b/drivers/staging/bcm/hostmibs.c > [] > > + S_MIBS_EXTSERVICEFLOW_PARAMETERS *t = &Adapter->PackInfo[uiSearchRuleIndex].stMibsExtServiceFlowTable; > > + > > + t->wmanIfSfid = psfLocalSet->u32SFID; > > + t->wmanIfCmnCpsMaxSustainedRate = psfLocalSet->u32MaxSustainedTrafficRate; > > + t->wmanIfCmnCpsMaxTrafficBurst = psfLocalSet->u32MaxTrafficBurst; > > + t->wmanIfCmnCpsMinReservedRate = psfLocalSet->u32MinReservedTrafficRate; > > + t->wmanIfCmnCpsToleratedJitter = psfLocalSet->u32ToleratedJitter; > > + t->wmanIfCmnCpsMaxLatency = psfLocalSet->u32MaximumLatency; > > + t->wmanIfCmnCpsFixedVsVariableSduInd = psfLocalSet->u8FixedLengthVSVariableLengthSDUIndicator; > > + t->wmanIfCmnCpsFixedVsVariableSduInd = ntohl(t->wmanIfCmnCpsFixedVsVariableSduInd); > > I suggest you don't a set and then use ntohl but > instead only use ntohl > > t->wmanIfCmnCpsFixedVsVariableSduInd = ntohl((long)psfLocalSet->u8FixedLengthVSVariableLengthSDUIndicator); > [] > > + t->wmanIfCmnCpsSfSchedulingType = psfLocalSet->u8ServiceFlowSchedulingType; > > + t->wmanIfCmnCpsSfSchedulingType = ntohl(t->wmanIfCmnCpsSfSchedulingType); > here > > + t->wmanIfCmnCpsArqEnable = psfLocalSet->u8ARQEnable; > > + t->wmanIfCmnCpsArqEnable = ntohl(t->wmanIfCmnCpsArqEnable); > here > > + t->wmanIfCmnCpsArqWindowSize = ntohs(psfLocalSet->u16ARQWindowSize); > > + t->wmanIfCmnCpsArqWindowSize = ntohl(t->wmanIfCmnCpsArqWindowSize); > > huh? There are a few of these that don't make much sense to me. > ntohs then ntohl? why? > > etc... > Joe, you're obviously right but these arent' bugs that Diego introduced so is it Ok if he sends fixes for those in a later patch? Also he's newbie with a @gmail.com address. He doesn't have the docs or the hardware and there is no way he knows what the ntohl(ntohs()) is about any more than we do... It would be good if we put a comment there though for future reference because you're right it's probably a bug. /* XXX Why are we doing a ntohl(ntohs()) here??? */ But I'd leave that code as is until someone has the hardware to test this. regards, dan carpenter
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
