> From: Roland Dreier [mailto:[EMAIL PROTECTED] > To: Christoph Hellwig; Glenn Streiff > > Rather than arguing over whether we have to have sparse clean code, I > decided to annotate the code myself. Here's a patch that fixes most > of the sparse warnings in the nes driver. There's still some stuff > that actually looks buggy, like the way hte_index stuff is handled. > You initialize hte_index_mask as: > > hte_index_mask = ((u32)1 << ((u32temp & 0x001f)+1))-1; > nesadapter->hte_index_mask = hte_index_mask; > > but then compute hte_index stuff with: > > nesqp->hte_index = cpu_to_be32( > crc32c(~0, (void *)&nes_quad, > sizeof(nes_quad)) ^ 0xffffffff); > > and then do: > > nesqp->hte_index &= nesadapter->hte_index_mask; > > which seems odd to say the least (hte_index is big-endian, > hte_index_mask is cpu-endian). > > And also, there's code with the loc_addr/rem_addr etc that seem very > confused. For example > > cm_info->loc_addr = htonl(cm_info->loc_addr); > cm_info->rem_addr = htonl(cm_info->rem_addr); > cm_info->loc_port = htons(cm_info->loc_port); > cm_info->rem_port = htons(cm_info->rem_port); > > which is obviously impossible to annotate correctly, and I couldn't > keep track of the endianness stuff elsewhere.
Thanks for the additional review and patch. I take your point. The part is little endian and the driver is functional for little and big endian platforms. There may have been some expedience with the declarations there. I think it can be improved. Let me take it up with the person who wrote that code. Also, I want everyone to understand that my skill set is weighted more towards build/install/config. And I guess I'll be patch wrangling as well. So I'll rely on input from my developers for issues that drill down or I'll have them post directly. I respect the work you guys do. For now, let me get some qa cycles with your patch across x86_64 and power (and probably a couple others). Regards, Glenn > > Anyway this is what I have in case the promised cleanups don't turn up > in time... > > Signed-off-by: Roland Dreier <[EMAIL PROTECTED]> > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
