On 2016-11-21 at 10:02 Gan Shun Lim (Gerrit) wrote: > Gan Shun Lim has posted comments on this change. ( > https://akaros-review.googlesource.com/3123 ) > > Change subject: dune: clean up and remove lots of cruft > ...................................................................... > > > Patch Set 3: Code-Review+2 > >
Merged to master at d7aa19a78ac2..19fd49b2e99e (from, to] You can see the entire diff with 'git diff' or at https://github.com/brho/akaros/compare/d7aa19a78ac2...19fd49b2e99e barret p.s. Yikes, you can see how nasty vmrunkernel.c is by the output of checkpatch on the initial copy to dune.c: $ git gerrit-track-review 3123 ron Gerrit remote branch is: refs/changes/23/3123/4 The review can be found at: https://akaros-review.googlesource.com/#/c/3123/ remote: Counting objects: 1960, done remote: Finding sources: 100% (17/17) remote: Total 17 (delta 8), reused 14 (delta 8) Unpacking objects: 100% (17/17), done. >From https://akaros.googlesource.com/akaros * branch refs/changes/23/3123/4 -> FETCH_HEAD The local branch is now at ron ../patches/0001-user-vmm-print-the-RSP-as-well-as-RIP.patch ../patches/0002-dune-add-a-dune-command.patch ../patches/0003-dune-clean-up-and-remove-lots-of-cruft.patch ----------------------------------------------------------- ../patches/0001-user-vmm-print-the-RSP-as-well-as-RIP.patch ----------------------------------------------------------- total: 0 errors, 0 warnings, 7 lines checked ../patches/0001-user-vmm-print-the-RSP-as-well-as-RIP.patch has no obvious style problems and is ready for submission. --------------------------------------------- ../patches/0002-dune-add-a-dune-command.patch --------------------------------------------- ERROR: trailing whitespace #51: FILE: tests/dune/Makefrag:11: +DUNE_TESTS_LDDEPENDS := $(DUNE_TESTS_DIR)/%.c $ WARNING: line over 80 characters #173: FILE: tests/dune/dune.c:102: +struct acpi_madt_local_apic Apic0 = {.header = {.type = ACPI_MADT_TYPE_LOCAL_APIC, .length = sizeof(struct acpi_madt_local_apic)}, WARNING: line over 80 characters #175: FILE: tests/dune/dune.c:104: +struct acpi_madt_io_apic Apic1 = {.header = {.type = ACPI_MADT_TYPE_IO_APIC, .length = sizeof(struct acpi_madt_io_apic)}, WARNING: line over 80 characters #191: FILE: tests/dune/dune.c:120: + * where the source entry is ‘0’ and the Global System Interrupt is ‘2.’ ERROR: do not initialise globals to 0 or NULL #198: FILE: tests/dune/dune.c:127: +volatile int shared = 0; WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #198: FILE: tests/dune/dune.c:127: +volatile int shared = 0; ERROR: do not initialise globals to 0 or NULL #199: FILE: tests/dune/dune.c:128: +volatile int quit = 0; WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #199: FILE: tests/dune/dune.c:128: +volatile int quit = 0; ERROR: do not initialise globals to 0 or NULL #215: FILE: tests/dune/dune.c:144: +int debug = 0; ERROR: do not initialise globals to 0 or NULL #216: FILE: tests/dune/dune.c:145: +int resumeprompt = 0; ERROR: need consistent spacing around '*' (ctx:WxV) #221: FILE: tests/dune/dune.c:150: +void vapic_status_dump(FILE *f, void *vapic); ^ WARNING: externs should be avoided in .c files #221: FILE: tests/dune/dune.c:150: +void vapic_status_dump(FILE *f, void *vapic); WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #226: FILE: tests/dune/dune.c:155: +#define BITOP_ADDR(x) "+m" (*(volatile long *) (x)) WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #231: FILE: tests/dune/dune.c:160: +static inline int test_and_set_bit(int nr, volatile unsigned long *addr); WARNING: Missing a blank line after declarations #240: FILE: tests/dune/dune.c:169: + uint32_t initial_count; + while (1) { ERROR: do not initialise globals to 0 or NULL #252: FILE: tests/dune/dune.c:181: +volatile int consdata = 0; WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #252: FILE: tests/dune/dune.c:181: +volatile int consdata = 0; ERROR: Bad function definition - void lowmem() should probably be void lowmem(void) #360: FILE: tests/dune/dune.c:289: +void lowmem() { ERROR: open brace '{' following function declarations go on the next line #360: FILE: tests/dune/dune.c:289: +void lowmem() { WARNING: line over 80 characters #361: FILE: tests/dune/dune.c:290: + __asm__ __volatile__ (".section .lowmem, \"aw\"\n\tlow: \n\t.=0x1000\n\t.align 0x100000\n\t.previous\n"); WARNING: unnecessary whitespace before a quoted newline #361: FILE: tests/dune/dune.c:290: + __asm__ __volatile__ (".section .lowmem, \"aw\"\n\tlow: \n\t.=0x1000\n\t.align 0x100000\n\t.previous\n"); WARNING: Missing a blank line after declarations #368: FILE: tests/dune/dune.c:297: + uint8_t *end = buffer + length; + fprintf(stderr, "tbchecksum %p for %d", buffer, length); ERROR: return is not a function, parentheses are not required #375: FILE: tests/dune/dune.c:304: + return (sum); WARNING: line over 80 characters #383: FILE: tests/dune/dune.c:312: + fprintf(stderr, "gencsum %p target %p source %d bytes\n", target, data, len); WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #390: FILE: tests/dune/dune.c:319: +static inline int test_and_set_bit(int nr, volatile unsigned long *addr) WARNING: line over 80 characters #619: FILE: tests/dune/dune.c:548: + fprintf(stderr, "read failed len was : %d, num_read was: %d\n", WARNING: Prefer ARRAY_SIZE(long_options) #633: FILE: tests/dune/dune.c:562: + i < sizeof(long_options)/sizeof(long_options[0]) - 1; WARNING: braces {} are not necessary for single statement blocks #643: FILE: tests/dune/dune.c:572: + if (strlen(cmdline_default) == 0) { + fprintf(stderr, "WARNING: No command line parameter file specified.\n"); + } WARNING: line over 80 characters #677: FILE: tests/dune/dune.c:606: + // And, sorry, due to the STUPID format of the RSDP for now we need the low 1M. WARNING: line over 80 characters #690: FILE: tests/dune/dune.c:619: + if ((csum = acpi_tb_checksum((uint8_t *) r, ACPI_RSDP_CHECKSUM_LENGTH)) != 0) { ERROR: do not use assignment in if condition #690: FILE: tests/dune/dune.c:619: + if ((csum = acpi_tb_checksum((uint8_t *) r, ACPI_RSDP_CHECKSUM_LENGTH)) != 0) { ERROR: do not use assignment in if condition #754: FILE: tests/dune/dune.c:683: + if ((csum = acpi_tb_checksum((uint8_t *) x, x->header.length)) != 0) { ERROR: space required before the open parenthesis '(' #905: FILE: tests/dune/dune.c:834: + for(uintptr_t p4 = memstart; p4 < memstart + memsize; WARNING: line over 80 characters #911: FILE: tests/dune/dune.c:840: + for (uintptr_t p2 = p3; p2 < memstart + memsize; p2 += PML2_PTE_REACH) { WARNING: line over 80 characters #918: FILE: tests/dune/dune.c:847: + fprintf(stderr, "p512 %p p512[0] is 0x%lx p1 %p p1[0] is 0x%x\n", p512, p512[0], p1, p1[0]); total: 13 errors, 22 warnings, 892 lines checked NOTE: Whitespace errors detected. You may wish to use scripts/cleanpatch or scripts/cleanfile ../patches/0002-dune-add-a-dune-command.patch has style problems, please review. ------------------------------------------------------------ ../patches/0003-dune-clean-up-and-remove-lots-of-cruft.patch ------------------------------------------------------------ ERROR: do not initialise statics to 0 or NULL #331: FILE: tests/dune/dune.c:43: +static int debug = 0; total: 1 errors, 0 warnings, 692 lines checked ../patches/0003-dune-clean-up-and-remove-lots-of-cruft.patch has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. -- 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.
