> Looking at the patches what you did seems OK. > > > But regarding "review" I have a different criticism directed > at Roland: > > This driver should really have gotten some review before > being included > in the kernel. > > Even a simple checkpatch run finds more than > 250 stylistic errors > (not code bugs but cases where the driver violates the standard code > formatting rules of kernel code). > > And I'm not talking about the > 2000 checkpatch warnings that > are mostly > about too long lines (which should arguably also be fixed). > > And many more issues that could have been foung during a review. > E.g. when you look at 3/8 from this series the code > if (!cm_node) > return -EINVAL; > new_send = kzalloc(sizeof(*new_send), GFP_ATOMIC); > if (!new_send) > return -1; > doesn't look good since the -1 should most likely better be something > like -ENOMEM (I haven't checked whether you can immediately change it > at this specific place). > > And these are just comments from someone with zero knowledge about > InfiniBand, but I'd expect InfiniBand-specifig bugs might be found > before they hit users if an InfiniBand maintainer would review the > complete driver. > > Note that this is not meant as a criticism against Glenn - it's > normal that submitted code contains bugs, but a code review > can help to > cope with this. > > > Glenn > > cu > Adrian >
Hi, Adrian. Yeah, I agree that the stylistic issues are annoying and I am actually itching to get some of those simples things corrected. Roland has outlined several areas for improvement in the driver (style-wise and substance-wise) and I'm working to address those. I'm learning the ropes here so I expect I'll get faster/better at responding and fixing things like the coverity issues you flagged. I need to pull these tools into my own release process so I'm catching flaws on my side. I want the driver to be worthy. Regards, Glenn
_______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
