Re: [PATCH net-next 1/2] wireless: Remove casts of void *
From: Julian Calaby <hidden>
Date: 2011-06-14 05:39:55
Also in:
lkml, netdev
Joe, On Tue, Jun 14, 2011 at 15:32, Joe Perches [off-list ref] wrote:
On Tue, 2011-06-14 at 15:23 +1000, Julian Calaby wrote:quoted
Joe,Hi Julian.quoted
On Tue, Jun 14, 2011 at 14:02, Joe Perches [off-list ref] wrote:quoted
Unnecessary casts of void * clutter the code. These are the remainder casts after several specific patches to remove netdev_priv and dev_priv.You seem to have removed a lot of casts that don't relate to these cleanups. In particular, some of the casts seem to relate more to documentation rather than just changing pointer types to make the compiler happy.All of the cast removals are casts of void* types. I think none of of the casts are useful. None of them are required, all are duplicative.
My issue here is that you mention in the commit log that this relates to the removal of netdev_priv and dev_priv, but the casts removed (mostly) don't.
quoted hunk ↗ jump to hunk
quoted
In particular, I'm referring to the casts describing the different usages of data_buf in mwiflex, and around some pointer math in ath9k.Can you describe more in detail why you think these are documentary? This sort of cast:diff --git a/drivers/net/wireless/mwifiex/11n.c b/drivers/net/wireless/mwifiex/11n.c@@ -191,7 +191,7 @@ int mwifiex_ret_11n_cfg(struct host_cmd_ds_command *resp, void *data_buf)struct host_cmd_ds_11n_cfg *htcfg = &resp->params.htcfg; if (data_buf) { - tx_cfg = (struct mwifiex_ds_11n_tx_cfg *) data_buf; + tx_cfg = data_buf; I think pretty useless. tx_cfg is a struct mwifiex_ds_11n_tx_cfg *.
True, but IMHO, this documents, particularly in a random snippet like this, what's going on. Arguably though, a better fix would be to move the cast to the place where this function is called.
quoted
Whilst I'm sure that the compiler is smart enough to handle automatic casts between pointer types, some of these, in particular the mwiflex bits, add some documentation to the code.I think not. Opinions of course can vary.
Of course. I rarely look at full files, so the more information that can be stuffed into a patch, the better for me. That said, I agree that all the casts removed are superfluous. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ .Plan: http://sites.google.com/site/juliancalaby/