Hi Johannes, > -----Original Message----- > From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] > Sent: Thursday, January 13, 2011 1:20 AM > To: Bing Zhao > Cc: linux-wireless@xxxxxxxxxxxxxxx; John W. Linville; Amitkumar Karwar; Kiran Divekar; Frank Huang > Subject: Re: [PATCH v2] mwifiex: remove linked list implementation > > On Wed, 2011-01-12 at 21:16 -0800, Bing Zhao wrote: > > Better, > > > + list_del((struct list_head *) tx_ba_tsr_tbl); > > but you shouldn't be casting at all. Do list_del(&tx_ba_tsr_tbl->list); > or so instead, so that this works when the list_head isn't the first > item in the struct. Thanks a lot for your comments. Will remove the casting. > > > - mwifiex_util_init_list((struct mwifiex_linked_list *) &priv-> > > - tx_ba_stream_tbl_ptr); > > + INIT_LIST_HEAD((struct list_head *) &priv->tx_ba_stream_tbl_ptr); > > Same here. Try to get rid of all casts. If you have any specific ones > that you don't know how to get rid of, feel free to ask. > > > - while (ra_list != (struct mwifiex_ra_list_tbl *) ra_list_hhead) { > > - if (mwifiex_util_peek_list(&ra_list->buf_head, true)) > > + int is_list_empty; > > + unsigned long flags; > > + > > + list_for_each_entry(ra_list, ra_list_hhead, list) { > > + spin_lock_irqsave(&ra_list->ra_list_tbl_lock, flags); > > + is_list_empty = list_empty(&ra_list->buf_head); > > + spin_unlock_irqrestore(&ra_list->ra_list_tbl_lock, flags); > > + if (!is_list_empty) > > return false; > > - > > - ra_list = (struct mwifiex_ra_list_tbl *) ra_list->next; > > } > > This seems rather dubious. If the list is empty, the for_each_entry > won't do anything? Or is that some other list? On each entry of the list, we do empty check for another list "buf_head". The structure looks like this: struct mwifiex_ra_list_tbl { struct list_head list; struct list_head buf_head; /* spin lock for ra list table */ spinlock_t ra_list_tbl_lock; ...... }; Thanks, Bing > > johannes ÿô.nlj·Ÿ®‰†+%ŠË±é¥Šwÿº{.nlj·¥Š{±ÿ«zW¬³ø¡Ü}©ž²ÆzÚj:+v‰¨þø®w¥þŠàÞ¨è&¢)ß«a¶Úÿûz¹ÞúŽŠÝjÿŠwèf
