Noted.

If checkpatch shows an error that is actually removed in a later commit,
what do I do?

On Tue, Dec 15, 2015 at 1:00 PM Barret Rhoden <[email protected]> wrote:

> On 2015-12-09 at 17:40 Gan Shun <[email protected]> wrote:
> > My newly rebased changes. There is no actual difference in the code
> > since Davide's changes did not affect me.
> >
> > The following changes since commit
> > 495a723da7db40d18c933c0d9b979058b0d8ffa4:
> >
> >   Use core_id_early() in kprof trace buffer print code (2015-12-08
> > 16:32:42 -0500)
> >
> > are available in the git repository at:
> >
> >   [email protected]:GanShun/akaros.git
> > c0c444f9646147a530672546d5cdbaf28e357e70
>
> In the future, can you create a branch at that commit on your repo and
> tell me the branch name?  I happened to be able to figure out it was
> iommu-g this time.  I'll keep tracking iommu-g for any updates to this
> patchset.
>
> There's also a few style warnings from checkpatch.  Some of them are
> probably false positives.
>
> For the block-commented ones, if you're trying to ignore a block of
> code, use #if 0 .. #endif.  Or just remove/fix it - odds are it's not a
> good solution long term.
>
> In the meantime, I'll start looking through what you've got so far.
>
> Here's my checkpatch output:
>
> ../patches/0001-Switched-from-APIC-to-X2APIC-mode.patch
> -------------------------------------------------------
> WARNING: quoted string split across lines
> #341: FILE: kern/arch/x86/apic9.c:162:
> +               printd("Ignoring read only register at offset 0x%02x in"
> +                      " X2APIC mode", r);
>
> total: 0 errors, 1 warnings, 469 lines checked
>
> ../patches/0001-Switched-from-APIC-to-X2APIC-mode.patch has style
> problems, please review.
> --------------------------------------------------------------------------
> ../patches/0002-Initialized-IOMMU-and-modified-IOAPIC-to-send-remapp.patch
> --------------------------------------------------------------------------
> WARNING: Block comments use * on subsequent lines
> #90: FILE: kern/arch/x86/ioapic.c:151:
> +       /*if (intin == 4) {
> +               hi = 4 << (49-32) | 1 << (48-32);
>
> WARNING: Block comments use * on subsequent lines
> #152: FILE: kern/arch/x86/ioapic.c:670:
> +       /*if (irq_h->dev_irq == 1) {
> +               rdt->hi = 1 << (49-32) | 1 << (48-32);
>
> WARNING: Missing a blank line after declarations
> #201: FILE: kern/arch/x86/iommu.c:15:
> +       uint64_t *irtarp;
> +       volatile uint32_t version, status;
>
> WARNING: Use of volatile is usually wrong: see
> Documentation/volatile-considered-harmful.txt
> #201: FILE: kern/arch/x86/iommu.c:15:
> +       volatile uint32_t version, status;
>
> WARNING: braces {} are not necessary for single statement blocks
> #205: FILE: kern/arch/x86/iommu.c:19:
> +       if (rootp == NULL) {
> +/*
> +               do {
>
> WARNING: Block comments use * on subsequent lines
> #270: FILE: kern/arch/x86/iommu.c:84:
> +/*
> +               do {
>
> WARNING: braces {} are not necessary for single statement blocks
> #304: FILE: kern/arch/x86/iommu.c:118:
> +       if (irtep[irte_index*2] & PRESENT) {
> +               panic("TRIED TO ALLOCATE ALREADY PRESENT IRTE");
> +       }
>
> WARNING: Use of volatile is usually wrong: see
> Documentation/volatile-considered-harmful.txt
> #385: FILE: kern/arch/x86/iommu.c:199:
> +       volatile uint32_t status;
>
> total: 0 errors, 10 warnings, 491 lines checked
>
> ../patches/0002-Initialized-IOMMU-and-modified-IOAPIC-to-send-remapp.patch
> has style problems, please review.
> --------------------------------------------------------------------------
> ../patches/0003-ExtINT-mode-for-IOMMU-is-working-and-remaps-IOAPIC-I.patch
> --------------------------------------------------------------------------
> total: 0 errors, 0 warnings, 132 lines checked
>
> ../patches/0003-ExtINT-mode-for-IOMMU-is-working-and-remaps-IOAPIC-I.patch
> has no obvious style problems and is ready for submission.
> --------------------------------------------------------------------------
> ../patches/0004-IOMMU-Rerouting-of-Interrupt-4-is-now-working.-Busyb.patch
> --------------------------------------------------------------------------
> WARNING: externs should be avoided in .c files
> #41: FILE: kern/arch/x86/apic9.c:124:
> +void __apic_ir_dump(uint64_t r);
>
> total: 0 errors, 1 warnings, 93 lines checked
>
> ../patches/0004-IOMMU-Rerouting-of-Interrupt-4-is-now-working.-Busyb.patch
> has style problems, please review.
> --------------------------------------------------------------------------
> ../patches/0005-Swapped-IPI-sending-to-a-full-64-bit-write-and-APIC-.patch
> --------------------------------------------------------------------------
> WARNING: braces {} are not necessary for any arm of this statement
> #385: FILE: kern/arch/x86/apic9.c:197:
> +       if (r == 0x31) {
> [...]
> +       } else if (r == 0x30) {
> [...]
>         } else
> [...]
>
> WARNING: braces {} are not necessary for single statement blocks
> #642: FILE: kern/arch/x86/trap.c:520:
> +       if (hw_tf->tf_trapno != I_KERNEL_MSG && core_id() != 0) {
> +               printk("I_KERNEL_MSG: core %d\n", core_id());
> +       }
>
> total: 0 errors, 2 warnings, 617 lines checked
>
> ../patches/0005-Swapped-IPI-sending-to-a-full-64-bit-write-and-APIC-.patch
> has style problems, please review.
>
> Barret
>
> --
> 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.
>

-- 
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.

Reply via email to