I actually slotted the changes in the same commit stream.
On Wed, Nov 11, 2015 at 9:19 AM, Davide Libenzi <[email protected]> wrote: > On Wed, Nov 11, 2015 at 8:43 AM, Barret Rhoden <[email protected]> > wrote: > >> Please run checkpatch on your patches before sending them in. A bunch >> of these are false positives, but many are not. >> > > Let me take a look ... > > > my checkpatch output: >> >> -------------------------------------------------------------------------- >> ../patches/0001-Completely-restructured-profiler-code-cutting-all-th.patch >> -------------------------------------------------------------------------- >> ERROR: space required after that ',' (ctx:VxV) >> #86: FILE: kern/drivers/dev/kprof.c:406: >> + n = profiler_read(va,n); >> > > Final code (tip of HEAD) does not have that anymore. > > > >> ^ >> >> ERROR: space required after that ',' (ctx:VxV) >> #161: FILE: kern/include/profiler.h:15: >> +int profiler_read(void *va,int); >> > > Ditto. > > > >> ^ >> >> ERROR: space required before the open parenthesis '(' >> #1233: FILE: kern/src/profiler.c:202: >> + for(core = 0; core < num_cores; core++) { >> > > Ditto. > > > ERROR: do not initialise statics to 0 or NULL >> #41: FILE: kern/src/profiler.c:33: >> +static int profiler_users = 0; >> > > Fixing now. > > > WARNING: kfree(NULL) is safe and this check is probably not required >> #108: FILE: kern/lib/circular_buffer.c:33: >> + if (cb->mem) >> + kfree(cb->mem); >> > > Existing code, not mine, that now is gone, BTW. > > > #320: FILE: kern/drivers/dev/kprof.c:56: >> + {".", {Kprofdirqid, 0, QTDIR},0, >> DMDIR|0550}, >> ^ >> > > Existing code, fixing now. > > > > >> >> WARNING: externs should be avoided in .c files >> #329: FILE: kern/drivers/dev/kprof.c:65: >> +extern int booting; >> > > Yes, this is an existing one, I can clean up, but please, do not hold this > CRs any further for stuff like this. > I will post a follow up patches for things like these. > > > > >> >> WARNING: kfree(NULL) is safe and this check is probably not required >> #413: FILE: kern/drivers/dev/kprof.c:132: >> + if (kprof.pdata) { >> + kfree(kprof.pdata); >> > > It does not only kfree, but anyway, fixing this as well. > > > >> >> WARNING: Missing a blank line after declarations >> #546: FILE: kern/drivers/dev/kprof.c:211: >> + int i; >> + ERRSTACK(1); >> > > False positive. BTW, empty like after var decls is great for me (my > standard), but 99% of existing code place absolutely no spacing, not only > between vars and code, but also between logical groups on instructions. > > > > >> >> ERROR: spaces required around that ':' (ctx:VxW) >> #615: FILE: kern/drivers/dev/kprof.c:264: >> + return kprof.pdata != NULL ? kprof.psize: profiler_size(); > > ^ >> > > OK, fixing. > > > >> >> ERROR: space required before the open brace '{' >> #646: FILE: kern/drivers/dev/kprof.c:291: >> + if (c->qid.type & QTDIR){ >> >> ERROR: "foo* const * bar" should be "foo * const *bar" >> #816: FILE: kern/drivers/dev/kprof.c:420: >> + const char* const * cmds = profiler_configure_cmds(); >> > > Fixing. > > > >> >> ERROR: Macros with complex values should be enclosed in parentheses >> #1191: FILE: kern/include/kdebug.h:51: >> +#define trace_printx(args...) if (printx_on) trace_printk(TRUE, args) >> > > Existing code, fixing now. > > > >> >> ERROR: "foo* const *bar" should be "foo * const *bar" >> #1242: FILE: kern/include/profiler.h:17: >> +const char* const *profiler_configure_cmds(void); >> > > Fixing. > > > >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1283: FILE: kern/include/ros/profiler_records.h:17: >> +} __attribute__((packed)); >> > > ENOENT > > > ERROR: "foo* bar" should be "foo *bar" >> #1335: FILE: kern/include/string.h:27: >> +void *memchr(const void* mem, int chr, int len); >> > > Existing code (a bunch there use the same syntax), fixing that only. > > > ERROR: do not initialise statics to 0 or NULL >> #1433: FILE: kern/src/profiler.c:40: >> +static int tracing = 0; >> > > Dupe, fixing. > > > >> >> ERROR: "foo* bar" should be "foo *bar" >> #1442: FILE: kern/src/profiler.c:50: >> +static inline char* vb_encode_uint64(char* data, uint64_t n) >> > > Fixing. > > >> >> ERROR: "foo* bar" should be "foo *bar" >> #1442: FILE: kern/src/profiler.c:50: >> +static inline char* vb_encode_uint64(char* data, uint64_t n) >> > > Dupe. > > > >> >> WARNING: kfree(NULL) is safe and this check is probably not required >> #1743: FILE: kern/src/profiler.c:278: >> + if (profiler_percpu_ctx) { >> + kfree(profiler_percpu_ctx); >> > > Fixing... > > > >> >> ERROR: else should follow close brace '}' >> #1812: FILE: kern/src/profiler.c:330: >> + } >> + else >> > > Fixing. > > > >> >> ERROR: "foo* const *bar" should be "foo * const *bar" >> #1819: FILE: kern/src/profiler.c:336: >> +const char* const *profiler_configure_cmds(void) >> > > Dupe. > > > >> >> ERROR: spaces required around that ':' (ctx:VxW) >> #1887: FILE: kern/src/profiler.c:387: >> + cpu_buf->tracing = onoff ? 1: -1; >> > > Fixing ... > > > >> ^ >> >> ERROR: spaces required around that ':' (ctx:VxW) >> #2001: FILE: kern/src/profiler.c:448: >> + return profiler_queue ? qlen(profiler_queue): 0; >> > > Fixing ... > > > >> ^ >> >> ERROR: spaces required around that ':' (ctx:VxW) >> #2007: FILE: kern/src/profiler.c:453: >> + return profiler_queue ? qread(profiler_queue, va, n): 0; >> > > Fixing ... > > > >> ^ >> >> ERROR: "foo* bar" should be "foo *bar" >> #2036: FILE: kern/src/string.c:121: >> +memchr(const void* mem, int chr, int len) >> > > Existing code, fixing ... > > > >> >> total: 16 errors, 8 warnings, 1975 lines checked >> >> ../patches/0013-Implemented-the-new-profiler.patch has style problems, >> please review. >> -------------------------------------------------------------------------- >> ../patches/0014-Enabled-prof-kptrace-collection-of-anything-which-go.patch >> -------------------------------------------------------------------------- >> WARNING: externs should be avoided in .c files >> #34: FILE: kern/drivers/dev/kprof.c:66: >> +extern system_timing_t system_timing; >> >> ERROR: space required after that ',' (ctx:VxV) >> #64: FILE: kern/include/stdarg.h:13: >> +#define va_copy(d,s) __builtin_va_copy(d,s) >> ^ >> >> ERROR: space required after that ',' (ctx:VxV) >> #64: FILE: kern/include/stdarg.h:13: >> +#define va_copy(d,s) __builtin_va_copy(d,s) >> > > Following existing code syntax. Changed now. > > > > >> ^ >> >> total: 2 errors, 1 warnings, 56 lines checked >> >> ../patches/0014-Enabled-prof-kptrace-collection-of-anything-which-go.patch >> has style problems, please review. >> --------------------------------------------------- >> ../patches/0015-Added-kprof-to-perf-converter.patch >> --------------------------------------------------- >> WARNING: line over 80 characters >> #159: FILE: tools/profile/kprof2perf/kprof2perf.c:45: >> + fprintf(stderr, "%s: %d: Assertion failed: " #c >> "\n", __FILE__, __LINE__); \ >> > > Fixing ... > > > >> >> ERROR: do not initialise statics to 0 or NULL >> #201: FILE: tools/profile/kprof2perf/kprof2perf.c:87: >> +static int debug_level = 0; >> > > Fixing ... > > > >> >> ERROR: do not initialise statics to 0 or NULL >> #204: FILE: tools/profile/kprof2perf/kprof2perf.c:90: >> +static struct static_mmap64 *static_mmaps = NULL; >> > > Fixing ... > > > >> >> ERROR: "foo* bar" should be "foo *bar" >> #211: FILE: tools/profile/kprof2perf/kprof2perf.c:97: >> +static inline const char* vb_decode_uint64(const char* data, uint64_t >> *pval) >> >> ERROR: "foo* bar" should be "foo *bar" >> #211: FILE: tools/profile/kprof2perf/kprof2perf.c:97: >> +static inline const char* vb_decode_uint64(const char* data, uint64_t >> *pval) >> > > Buy one, get two. Fixing now ... > > > > >> >> WARNING: line over 80 characters >> #352: FILE: tools/profile/kprof2perf/kprof2perf.c:238: >> +static char *mem_block_write(struct mem_block *mb, const void *data, >> size_t size) >> > > Fixing ... > > > >> >> ERROR: spaces required around that ':' (ctx:VxW) >> #373: FILE: tools/profile/kprof2perf/kprof2perf.c:259: >> + return (flags & MBWR_SOLID) ? (space >= size): (space > 0); >> > > Fixing ... > > > >> ^ >> >> WARNING: line over 80 characters >> #560: FILE: tools/profile/kprof2perf/kprof2perf.c:446: >> + struct proftype_kern_trace64 *rec = (struct proftype_kern_trace64 >> *) pr->data; >> >> WARNING: line over 80 characters >> #583: FILE: tools/profile/kprof2perf/kprof2perf.c:469: >> + struct proftype_user_trace64 *rec = (struct proftype_user_trace64 >> *) pr->data; >> > > Fixing both ... > > > >> >> ERROR: "foo* bar" should be "foo *bar" >> #619: FILE: tools/profile/kprof2perf/kprof2perf.c:505: >> + const uint64_t* ids, >> size_t nids) >> > > Fixing ... > > > >> >> WARNING: please, no space before tabs >> #890: FILE: tools/profile/kprof2perf/perf_format.h:85: >> +^I * ^I{ u32^I^I^Ipid, tid; } && PERF_SAMPLE_TID$ >> >> WARNING: please, no space before tabs >> #891: FILE: tools/profile/kprof2perf/perf_format.h:86: >> +^I * ^I{ u64^I^I^Itime; } && PERF_SAMPLE_TIME$ >> >> WARNING: please, no space before tabs >> #892: FILE: tools/profile/kprof2perf/perf_format.h:87: >> +^I * ^I{ u64^I^I^Iid; } && PERF_SAMPLE_ID$ >> >> WARNING: please, no space before tabs >> #893: FILE: tools/profile/kprof2perf/perf_format.h:88: >> +^I * ^I{ u64^I^I^Istream_id;} && PERF_SAMPLE_STREAM_ID$ >> >> WARNING: please, no space before tabs >> #894: FILE: tools/profile/kprof2perf/perf_format.h:89: >> +^I * ^I{ u32^I^I^Icpu, res; } && PERF_SAMPLE_CPU$ >> >> WARNING: please, no space before tabs >> #924: FILE: tools/profile/kprof2perf/perf_format.h:119: >> +^I * ^Istruct sample_id^I^Isample_id;$ >> >> WARNING: please, no space before tabs >> #935: FILE: tools/profile/kprof2perf/perf_format.h:130: >> +^I * ^Istruct sample_id^I^Isample_id;$ >> >> WARNING: please, no space before tabs >> #946: FILE: tools/profile/kprof2perf/perf_format.h:141: >> +^I * ^Istruct sample_id^I^Isample_id;$ >> >> WARNING: please, no space before tabs >> #957: FILE: tools/profile/kprof2perf/perf_format.h:152: >> +^I * ^Istruct sample_id^I^Isample_id;$ >> >> WARNING: please, no space before tabs >> #969: FILE: tools/profile/kprof2perf/perf_format.h:164: >> +^I * ^Istruct sample_id^I^Isample_id;$ >> >> WARNING: please, no space before tabs >> #980: FILE: tools/profile/kprof2perf/perf_format.h:175: >> +^I * ^Istruct sample_id^I^Isample_id;$ >> >> WARNING: please, no space before tabs >> #1027: FILE: tools/profile/kprof2perf/perf_format.h:222: >> +^I * ^I{ u64^I^I^Iabi; # enum perf_sample_regs_abi$ >> >> WARNING: please, no space before tabs >> #1028: FILE: tools/profile/kprof2perf/perf_format.h:223: >> +^I * ^I u64^I^I^Iregs[weight(mask)]; } && PERF_SAMPLE_REGS_USER$ >> >> WARNING: please, no space before tabs >> #1030: FILE: tools/profile/kprof2perf/perf_format.h:225: >> +^I * ^I{ u64^I^I^Isize;$ >> >> WARNING: please, no space before tabs >> #1031: FILE: tools/profile/kprof2perf/perf_format.h:226: >> +^I * ^I char^I^I^Idata[size];$ >> >> WARNING: please, no space before tabs >> #1032: FILE: tools/profile/kprof2perf/perf_format.h:227: >> +^I * ^I u64^I^I^Idyn_size; } && PERF_SAMPLE_STACK_USER$ >> >> WARNING: please, no space before tabs >> #1059: FILE: tools/profile/kprof2perf/perf_format.h:254: >> +^I * ^Istruct sample_id^I^Isample_id;$ >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1223: FILE: tools/profile/kprof2perf/perf_format.h:418: >> +} __attribute__((packed)); >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1249: FILE: tools/profile/kprof2perf/perf_format.h:444: >> +} __attribute__((packed)); >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1254: FILE: tools/profile/kprof2perf/perf_format.h:449: >> +} __attribute__((packed)); >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1295: FILE: tools/profile/kprof2perf/perf_format.h:490: >> +} __attribute__((packed)); >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1307: FILE: tools/profile/kprof2perf/perf_format.h:502: >> +} __attribute__((packed)); >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1316: FILE: tools/profile/kprof2perf/perf_format.h:511: >> +} __attribute__((packed)); >> >> WARNING: __packed is preferred over __attribute__((packed)) >> #1333: FILE: tools/profile/kprof2perf/perf_format.h:528: >> +} __attribute__((packed)); >> >> total: 6 errors, 28 warnings, 1291 lines checked >> > > This whole bunch coming from Linux. Figure that. Fixed now. > > > > >> >> ../patches/0015-Added-kprof-to-perf-converter.patch has style problems, >> please review. >> -------------------------------------------------------------------------- >> ../patches/0016-Added-patch-to-Linux-perf-to-allow-it-to-work-with-A.patch >> -------------------------------------------------------------------------- >> ERROR: trailing whitespace >> #33: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:14: >> + $ >> >> ERROR: trailing whitespace >> #37: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:18: >> + $ >> >> ERROR: trailing whitespace >> #45: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:26: >> + $ >> >> ERROR: trailing whitespace >> #51: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:32: >> + $ >> >> ERROR: trailing whitespace >> #98: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:79: >> + $ >> >> ERROR: trailing whitespace >> #102: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:83: >> + $ >> >> ERROR: trailing whitespace >> #104: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:85: >> + $ >> >> ERROR: trailing whitespace >> #116: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:97: >> + $ >> >> ERROR: trailing whitespace >> #127: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:108: >> + $ >> >> ERROR: trailing whitespace >> #133: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:114: >> + $ >> >> ERROR: trailing whitespace >> #144: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:125: >> + $ >> >> ERROR: trailing whitespace >> #149: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:130: >> + $ >> >> ERROR: trailing whitespace >> #165: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:146: >> + $ >> >> ERROR: trailing whitespace >> #167: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:148: >> + $ >> >> ERROR: trailing whitespace >> #173: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:154: >> + $ >> >> ERROR: trailing whitespace >> #177: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:158: >> + $ >> >> ERROR: trailing whitespace >> #183: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:164: >> + $ >> >> ERROR: trailing whitespace >> #188: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:169: >> + $ >> >> ERROR: trailing whitespace >> #192: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:173: >> + $ >> >> ERROR: trailing whitespace >> #212: FILE: tools/profile/kprof2perf/perf_patches/perf_patch.diff:193: >> + $ >> >> total: 20 errors, 0 warnings, 201 lines checked >> > > This is a patch file. It is going to fail. > > > WARNING: please write a paragraph that describes the config symbol fully >> #22: FILE: kern/src/ktest/Kconfig.postboot:120: >> +config TEST_circular_buffer >> > > Does not apply. > > > I will fix those in a follow commit on that branch. > > -- You received this message because you are subscribed to the Google Groups "Akaros" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/d/optout.
