* Holger Schurig | 2008-04-18 15:10:53 [+0200]: >> > I wrote "if (ret) return ret", not "if (ret) return;". >> >> I know you did, but I don't know why. The >> lbs_ethtool_get_stats() function returns void. Sebastian's >> patch which memsets it to zero makes sense, but it can't >> return an error. > >Ahh, I stand corrected. The function before lbs_ethtool_get_stats() (in net code that determinate the number of elements, dunno the name right now) is able to return an error. This one could check if the firmware has mesh support. >> They're separate problems, really. With or without the patch I >> first posted, you're getting crap back when you ask for >> statistics on a non-mesh device. The second patch fixes that, >> and stands alone. Do I understand this correct: If the firmware has support for mesh devices than we have ethX and mshX and only mshX should return the statistics? >Yes, this is separate. That's why I wrote "The ultimate patch >would be". > >BTW, I like your two patches combined more than Sebastians memset >approach. Because with Sebastians patch on on ethX we would get >zeroed nonsense instead of uninitialized nonsense. returning uninitialized nonsense is leaking kernel memory. >The memset should possible there anyway in the case of a firmware >that can mshX, but not CMD_ACT_MESH_GET_STATS or which would >return some error for another reason. What about changing lbs_ethtool_get_stats() from void to int? All other drivers are reading memory to get this data, we have to go through usb/cs/sdio layer and all of them may fail. I will try to form a patch around Monday that fixes Dan's comments (if nobody else is going to). Sebastian -- 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
