Search Linux Wireless

Re: [PATCH 01/83] staging: brcm80211: removed unused Broadcom specific ioctls codes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Roland,

I've passed an eye over the entire patch set.

My comments are pretty minor, but I've replied to the patches that I
have concerns for. Please note that I'm not really a developer, and my
comments are mostly style issues, not actual functional issues. Oh,
and also, I read most of the patches out of order, so some of the
comments may not make complete sense.

Overall, my comments are these:
1. If you're doing typedef removals in a patch, mention it in the summary.
2. Work on your summaries, there are a few cases where I think they're
a little too terse.
3. Good work!
4. Keep going =)

A couple of items you might want to add to your TODO list:
1. Replace the REG_[RW] macros with inline functions or equivalent
because they're ugly.
2. You shouldn't need to #ifdef on BIG_ENDIAN - there are a set of
macros to help with working with different endian systems.
3. Do a typedef removal sweep over the code because typedefs are ugly =)
4. I had a glance at /Documentation/CodingStyle and saw that there
were some issues mentioned in there that could be fixed in your code.
You might also want to play with checkpatch.
5. Personally, I'd avoid void pointers, but I'm not sure of the exact
rules. Could someone else weigh in on this?

I'm probably missing a whole lot of other, much more important, stuff,
but this is what I saw.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@xxxxxxxxx
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux