On Sun, Feb 27, 2011 at 11:46:40PM +0100, Gábor Stefanik wrote: > 2011/2/27 Dan Carpenter <error27@xxxxxxxxx>: > > On Sun, Feb 27, 2011 at 11:30:10PM +0100, Gábor Stefanik wrote: > >> >> - lastframe = (fc & IEEE80211_FCTL_MOREFRAGS) == 0; > >> >> + lastframe = ieee80211_has_morefrags(h->frame_control); > >> > > >> > This is reversed. It should be: > >> > if (ieee80211_has_morefrags(h->frame_control)) > >> > lastframe = false; > >> > else > >> > lastframe = true; > >> > >> lastframe = !ieee80211_has_morefrags(h->frame_control); > >> > > > > That way is a bit uglier. The ! and the i sort of blend together and > > make it harder to read. > > Parenthesize it, then. Branching based on a boolean value, only to > assign a different value to a boolean variable on each branch looks > unprofessional. > > Perhaps even add an equality, check? Like this: > if (ieee80211_has_morefrags(h->frame_control) == true)... > Yeah. I obviously considered a several ways to write that and chose the one I prefered... *shrug*. Write it the way Gábor says, because he cares a lot more about this bike shed than I do. ;) regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
