lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <alpine.NEB.2.11.1502090901440.395@chris.i8u.org> Date: Mon, 9 Feb 2015 09:03:54 -0800 (PST) From: Hisashi T Fujinaka <htodd@...fifty.com> To: Bjørn Mork <bjorn@...k.no> cc: Jeff Kirsher <jeffrey.t.kirsher@...el.com>, linux.nics@...el.com, e1000-devel@...ts.sourceforge.net, bruce.w.allan@...el.com, jesse.brandeburg@...el.com, linux-kernel@...r.kernel.org, john.ronciak@...el.com, netdev@...r.kernel.org, Andrej Manduch <amanduch@...il.com> Subject: Re: [E1000-devel] [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210 On Mon, 9 Feb 2015, Bj?rn Mork wrote: > Jeff Kirsher <jeffrey.t.kirsher@...el.com> writes: > >> If you want to see Nick's patch, feel free to view his patch on >> my queue tree: >> https://git.kernel.org/cgit/linux/kernel/git/jkirsher/queue.git/ > > which said: > - s32 i = 0, timeout = 200; /* FIXME: find real value to use here */ > + s32 i = 0, timeout = 200; > > > Comments like the one deleted by that patch do have some value in my > opinion: They document that the constant is chosen arbitrarily and is > not taken from some datasheet. > > Not a big deal, but leaving such comments, even if the value never ever > changes, could save someone from trying to figure out where the heck you > got that constant. And there's noone actually misinterpreting this > comment as "something needs to be fixed here", is there? So the cost of > leaving the comment is exactly zero. > > Just my .02 ?. I'm not going to tell you how to maintain your driver, > of course. At least I try not to :-) I think, for now, we should NAK this. It's a short comment and you don't want me to replace it with a philosophical statement on how you could tune the timeout. That's what the data sheet is for. (Replying from my home account and not my work account - todd.fujinaka@...el.com) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists