Re: [Crash-utility] [PATCH RFC 00/14] Minor code cleanups, round zero
----- Original Message -----
I'll take a look at these when I get the chance, but I'm really
not particularly excited unless they are actual bugs.
Like this one:
--- a/memory.c
+++ b/memory.c
@@ -17003,8 +17003,8 @@ valid_section(ulong addr)
if ((mem_section = read_mem_section(addr)))
return (ULONG(mem_section +
- OFFSET(mem_section_section_mem_map)) &&
- SECTION_MARKED_PRESENT);
+ OFFSET(mem_section_section_mem_map))
+ & SECTION_MARKED_PRESENT);
return 0;
}
@@ -17016,7 +17016,7 @@ section_has_mem_map(ulong addr)
if ((mem_section = read_mem_section(addr)))
return (ULONG(mem_section +
OFFSET(mem_section_section_mem_map))
- && SECTION_HAS_MEM_MAP);
+ & SECTION_HAS_MEM_MAP);
return 0;
}
This code has been like this since the original CONFIG_SPARSEMEM support
patch was posted back in 2006. Interesting that this has never been a
problem. Apparently nobody's ever bumped into mem_section that didn't
have those flags.
And then there's this one:
--- a/x86_64.c
+++ b/x86_64.c
@@ -2086,7 +2086,7 @@ x86_64_kvtop(struct task_context *tc, ulong kvaddr, physaddr_t
*paddr, int verbo
}
start_vtop_with_pagetable:
- if (!(*pml4) & _PAGE_PRESENT)
+ if ((!(*pml4)) & _PAGE_PRESENT)
goto no_kpage;
pgd_paddr = (*pml4) & PHYSICAL_PAGE_MASK;
FILL_PGD(pgd_paddr, PHYSADDR, PAGESIZE());
@@ -2187,7 +2187,7 @@ x86_64_kvtop_xen_wpt(struct task_context *tc, ulong kvaddr,
physaddr_t *paddr, i
fprintf(fp, "PML4 DIRECTORY: %lx\n", vt->kernel_pgd[0]);
fprintf(fp, "PAGE DIRECTORY: %lx [machine]\n", *pml4);
}
- if (!(*pml4) & _PAGE_PRESENT)
+ if ((!(*pml4)) & _PAGE_PRESENT)
goto no_kpage;
pgd_paddr = (*pml4) & PHYSICAL_PAGE_MASK;
pgd_paddr = xen_m2p(pgd_paddr);
It's pretty much impossible to have a pml without its PAGE_PRESENT bit set,
so it's been a benign bug. But the fix is incorrect. They should be:
if (!(*pml4 & _PAGE_PRESENT))
And I'm not sure wheter this one should be fixed by just removing the statement:
--- a/sadump.c
+++ b/sadump.c
@@ -157,9 +157,6 @@ read_dump_header(char *file)
}
restart:
- if (block_size < 0)
- return FALSE;
-
if (!read_device(sph, block_size, &offset)) {
error(INFO, "sadump: cannot read partition header\n");
goto err;
because farther down there is this:
if (sh->block_size != block_size) {
block_size = sh->block_size;
offset = 0;
goto restart;
}
I'll let the sadump maintainer decide how to deal with this one. He's on
this list, but I've cc'd him to get his attention.
Thanks,
Dave