[Crash-utility] [PATCH v2] Remove the VOID_PTR facilitator macro

[Dave Anderson]

----- Original Message -----
> I've been wondering about the intended use of this facilitator macro
> for some time and concluded that it is plainly wrong. It should
> take a pointer value from a buffer, but what is the use of a pointer
> that pointed to something on the target machine?
> 
> Since it cannot be meaningfully dereferenced on the host, the only
> (arguable) advantage is the ability to do pointer arithmetics on the
> variables. This second patch does the correct thing wherever pointer
> arithmetics was done, making the actual calculations more explicit.
> 
> Signed-off-by: Petr Tesarik <ptesarik at suse.cz>

Everything *looks* correct -- but this patch doesn't fix anything, while
having the capacity to break what works.  The slab-related changes still
concern me, at least until I test them.

BTW, in the future, when you post patches can you add them as attachments
instead of inlining them?

Thanks,
  Dave

 
> ---
> memory.c | 47 +++++++++++++++++++++++++----------------------
> 1 file changed, 25 insertions(+), 22 deletions(-)
> 
> --- a/memory.c
> +++ b/memory.c
> @@ -26,8 +26,8 @@ struct meminfo { /* general pu
> ulong c_offset;
> ulong c_num;
> ulong s_mem;
> - void *s_freep;
> - ulong *s_index;
> + ulong s_freep;
> + ulong s_index;
> ulong s_inuse;
> ulong cpucached_cache;
> ulong cpucached_slab;
> @@ -139,7 +139,7 @@ static int next_kpage(ulong, ulong *);
> static ulong last_vmalloc_address(void);
> static ulong next_vmlist_vaddr(ulong);
> static int next_identity_mapping(ulong, ulong *);
> -static int vm_area_page_dump(ulong, ulong, ulong, ulong, void *,
> +static int vm_area_page_dump(ulong, ulong, ulong, ulong, ulong,
> struct reference *);
> static int dump_swap_info(ulong, ulong *, ulong *);
> static void swap_info_init(void);
> @@ -3138,7 +3138,7 @@ vm_area_dump(ulong task, ulong flag, ulo
> ulong vma;
> ulong vm_start;
> ulong vm_end;
> - void *vm_next, *vm_mm;
> + ulong vm_next, vm_mm;
> char *dentry_buf, *vma_buf, *file_buf;
> ulong vm_flags;
> ulong vm_file, inode;
> @@ -3202,7 +3202,7 @@ vm_area_dump(ulong task, ulong flag, ulo
> !DO_REF_SEARCH(ref))
> fprintf(fp, vma_header);
> 
> - for (found = FALSE; vma; vma = (ulong)vm_next) {
> + for (found = FALSE; vma; vma = vm_next) {
> 
> if ((flag & PHYSADDR) && !DO_REF_SEARCH(ref))
> fprintf(fp, "%s", vma_header);
> @@ -3211,9 +3211,9 @@ vm_area_dump(ulong task, ulong flag, ulo
> BZERO(buf1, BUFSIZE);
> vma_buf = fill_vma_cache(vma);
> 
> - vm_mm = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_mm));
> + vm_mm = ULONG(vma_buf + OFFSET(vm_area_struct_vm_mm));
> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
> vm_flags = SIZE(vm_area_struct_vm_flags) == sizeof(short) ?
> USHORT(vma_buf+ OFFSET(vm_area_struct_vm_flags)) :
> @@ -3335,7 +3335,7 @@ vm_area_page_dump(ulong vma,
> ulong task,
> ulong start,
> ulong end,
> - void *mm,
> + ulong mm,
> struct reference *ref)
> {
> physaddr_t paddr;
> @@ -3347,7 +3347,7 @@ vm_area_page_dump(ulong vma,
> char buf3[BUFSIZE];
> char buf4[BUFSIZE];
> 
> - if ((ulong)mm == symbol_value("init_mm"))
> + if (mm == symbol_value("init_mm"))
> return FALSE;
> 
> if (!ref || DO_REF_DISPLAY(ref))
> @@ -9720,9 +9720,9 @@ dump_slab(struct meminfo *si)
> return;
> }
> 
> - si->s_freep = VOID_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
> + si->s_freep = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_freep));
> si->s_inuse = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_inuse));
> - si->s_index = ULONG_PTR(si->slab_buf + OFFSET(kmem_slab_s_s_index));
> + si->s_index = ULONG(si->slab_buf + OFFSET(kmem_slab_s_s_index));
> s_offset = USHORT(si->slab_buf + OFFSET(kmem_slab_s_s_offset));
> 
> if (!(si->flags & ADDRESS_SPECIFIED)) {
> @@ -9850,7 +9850,7 @@ dump_slab_percpu_v2(struct meminfo *si)
> static void
> gather_slab_free_list(struct meminfo *si)
> {
> - ulong *next, obj;
> + ulong next, obj;
> ulong expected, cnt;
> 
> BNEG(si->addrlist, sizeof(ulong) * (si->c_num+1));
> @@ -9885,16 +9885,18 @@ gather_slab_free_list(struct meminfo *si
> */
> 
> if (si->c_flags & SLAB_CFLGS_BUFCTL)
> - obj = si->s_mem + ((next - si->s_index) * si->c_offset);
> + obj = si->s_mem +
> + ((next - si->s_index) / sizeof(ulong)
> + * si->c_offset);
> else
> - obj = (ulong)next - si->c_offset;
> + obj = next - si->c_offset;
> 
> si->addrlist[cnt] = obj;
> 
> if (si->flags & ADDRESS_SPECIFIED) {
> if (INSLAB(next, si) &&
> - (si->spec_addr >= (ulong)next) &&
> - (si->spec_addr < (ulong)(next + 1))) {
> + (si->spec_addr >= next) &&
> + (si->spec_addr < next + sizeof(ulong))) {
> si->found = KMEM_BUFCTL_ADDR;
> return;
> }
> @@ -9909,7 +9911,7 @@ gather_slab_free_list(struct meminfo *si
> si->errors++;
> }
> 
> - readmem((ulong)next, KVADDR, &next, sizeof(void *),
> + readmem(next, KVADDR, &next, sizeof(ulong),
> "s_freep chain entry", FAULT_ON_ERROR);
> } while (next);
> 
> @@ -10057,7 +10059,7 @@ static void
> dump_slab_objects(struct meminfo *si)
> {
> int i, j;
> - ulong *next;
> + ulong next;
> int on_free_list;
> ulong cnt, expected;
> ulong bufctl, obj;
> @@ -10088,7 +10090,8 @@ dump_slab_objects(struct meminfo *si)
> if (si->c_flags & SLAB_CFLGS_BUFCTL) {
> for (i = 0, next = si->s_index; i < si->c_num; i++, next++) {
> obj = si->s_mem +
> - ((next - si->s_index) * si->c_offset);
> + ((next - si->s_index) / sizeof(ulong)
> + * si->c_offset);
> DUMP_SLAB_OBJECT();
> }
> } else {
> @@ -11561,7 +11564,7 @@ next_upage(struct task_context *tc, ulon
> int found;
> char *vma_buf;
> ulong vm_start, vm_end;
> - void *vm_next;
> + ulong vm_next;
> 
> if (!tc->mm_struct)
> return FALSE;
> @@ -11575,12 +11578,12 @@ next_upage(struct task_context *tc, ulon
> 
> vaddr = VIRTPAGEBASE(vaddr) + PAGESIZE(); /* first possible page */
> 
> - for (found = FALSE; vma; vma = (ulong)vm_next) {
> + for (found = FALSE; vma; vma = vm_next) {
> vma_buf = fill_vma_cache(vma);
> 
> vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
> vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
> - vm_next = VOID_PTR(vma_buf + OFFSET(vm_area_struct_vm_next));
> + vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
> 
> if (vaddr <= vm_start) {
> *nextvaddr = vm_start;
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility