On 10/3/20 1:01 PM, Kalesh Singh wrote:
On Sat, Oct 3, 2020 at 3:13 AM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
...
A remaining nit: there are lots of > 80 col lines here. Only those lines
that contain printf output strings really need to be extra-long. The
others can all be easily made to fit.
Thanks for the review John. I can fix the lines that don't need to be
80 col. FWIW checkpatch now allows 100 col lines:
https://lore.kernel.org/r/9c360bfa43580ce7726dd3d9d247f1216a690ef0.camel@xxxxxxxxxxx
Yep, I watched that go by. The new guidelines match what I recommended here,
though. It's slightly more nuanced than any sort of hard limit at either
80 or 100 lines, as you know from reading that--I'm just saying that out
loud for the benefit of others who may see this thread.
...
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
Not really a comment on this patch, but: at least two other test suites
in selftests/ have had to recreate MIN() or min(), and probably a few
other common things as well. It would be nice to gather up some of these
common things into one of the shared header files in selftests/.
I'm certainly not saying that this patchset has that responsibility,
though. Just pointing it out in case someone has a moment to clean up a
few here and there.
Thanks for pointing this out. I'll leave it as a task for a separate
patch if time permits or no one gets around to it before :-)
Sound great. :)
thanks,
--
John Hubbard
NVIDIA
