2017-09-24 13:42 GMT-04:00 Johannes Berg <johannes@xxxxxxxxxxxxxxxx>: > On Sun, 2017-09-24 at 13:21 -0400, Xiang Gao wrote: >> >> Do you mean to put more characters each line in the description >> > Huh, sorry, no - my bad. I was thinking of the code, not the > description at all. Oh yes, these indentation do looks ugly. Thank you for figuring this out. The tab width of my editor was set to 4. It should be 8... I will fix these problems and resend the patch soon, maybe after receiving a bit more feedback. > > For example here: > >> -int ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad, >> - u8 *data, size_t data_len, u8 *mic) >> +int aead_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, size_t aad_len, >> + u8 *data, size_t data_len, u8 *auth) >> > > I think you should adjust the indentation to match - or did it just get > mangled in my mail? It looks *further* indented now, when it should be > less (to after the opening parenthesis). Similarly in various other > places. > > And perhaps for long things like > >> +static inline struct crypto_aead *ieee80211_aes_key_setup_encrypt( >> + const u8 key[], size_t key_len, >> size_t mic_len) > >> +struct crypto_aead *aead_key_setup_encrypt(const char *alg, >> + const u8 key[], size_t key_len, size_t authsize); > > it might be better to write > > static inline struct crypto_aead * > ieee80211_aes_key_setup_encrypt(const u8 key[], ...) > > and > > struct crypto_aead * > aead_key_setup_encrypt(const char *alg, ...) > > > respectively, depending on how far you have to indent to break lines > etc. > > Anyway, I'm nitpicking. > > Unrelated to this, I'm not sure whose tree this should go through - > probably Herbert's (or DaveM's with his ACK? not sure if there's a > crypto tree?) or so? Yes, there is one at https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/ I'm not sure which tree to go either. I'm also not sure about the beginning of the patch title, should it be "mac80211:" or "crypto:"? Options are: 1. This whole patch goes to either mac80211 tree or crypto tree. I don't know which is better. 2. Make the higher level api only for internal usage in mac80211, i.e. move the aead_api.c and aead_api.h to net/mac80211, does not export the symbol. And of course, this will go to the mac80211 tree. I personally don't want this to be the final solution because I happen to be writing a loadable kernel module that uses these higher level api. 3. Maybe split this patch, one for changes in crypto, which will go to crypto tree, and the other for mac80211 part, which goes to the mac80211 tree? > > johannes
